feat(api_fastapi): support OR operator in search parameters #60008
Conversation
Jkhall81
requested review from
bbovenzi,
bugraoz93,
ephraimbuddy,
guan404ming,
jason810496,
pierrejeambrun,
rawwar,
ryanahamilton,
shubhamraj-git and
vatsrahul1001
as code owners
boring-cyborg
bot
added
area:API
|
There are two failures in the CI, but they appear to be unrelated to the code changes:
My code changes were in |
potiuk
force-pushed
the
features/fastapi-or-search-59545
branch
from
5616b93 to
c3dc23d
Compare
|
In case something like that happens - just rebase next time. ABR -> Always Be Rebased. |
jason810496
left a comment
jason810496
left a comment
There was a problem hiding this comment.
Thanks for the PR! LGTM overall.
Not sure whether @pierrejeambrun will have any comment for the current public interface change? I will defer to him for the final call.
| sql = str(statement.compile(compile_kwargs={"literal_binds": True})).lower() | ||
| assert "dag_id" in sql | ||
| assert "like" in sql | ||
|
|
||
| def test_to_orm_multiple_values_or(self): | ||
| """Test search with multiple terms using the pipe | operator.""" | ||
| param = _SearchParam(DagModel.dag_id).set_value("example_bash | example_python") | ||
| statement = select(DagModel) | ||
| statement = param.to_orm(statement) | ||
|
|
||
| sql = str(statement.compile(compile_kwargs={"literal_binds": True})) | ||
| assert "OR" in sql | ||
| assert "example_bash" in sql | ||
| assert "example_python" in sql |
There was a problem hiding this comment.
small nit: How about asserting the whole expected SQL statement as str for better readability?
There was a problem hiding this comment.
Thanks for the suggestion! I went with the keyword assertions because I thought asserting the full compiled SQL string might make the test brittle / requre maintenance in the future. If the SQLAlchemy version changes its formatting or if we run tests against different DB dialects, then a hardcoded string could cause failures. By checking for the presence of the OR operator and the specific search terms used, we can verify the logic and keep the test robust. What do you think?
|
Just wanted to say this is by far the coolest project I've contributed to so far. The pre-commit system and the automation you guys have set up is incredible. It's been a great learning experience. Also really appreciate how active and responsive the community is here! |
3 participants
feat(api_fastapi): support OR operator in search parameters
closes: #59545
Summary
This PR introduces support for the
ORoperator using the pipe (|) character in the FastAPI-based Airflow 3.0 API. By updating the centralized_SearchParamlogic, this functionality is enabled across multiple entities including DAGs, Dag Runs, and Task Instances.Changes
_SearchParam.to_orminparameters.pyto parse search terms separated by|and wrap them in asqlalchemy.or_condition.DESCRIPTIONinsearch_param_factoryto inform users of the new syntax in the API documentation.test_parameters.pyto verify SQL generation for both single-term and multi-term searches.