Usiel commented on code in PR #32709:
URL: https://github.com/apache/airflow/pull/32709#discussion_r1282817431
##########
airflow/api/client/api_client.py:
##########
@@ -60,12 +60,13 @@ def get_pools(self):
"""Get all pools."""
raise NotImplementedError()
- def create_pool(self, name, slots, description):
+ def create_pool(self, name, slots, description, include_deferred):
Review Comment:
Internally we cover all our bases upstream of the client, unless I'm missing
something. The `pool_command` module is the only consumer and in both usages we
handle a missing `include_deferred`:
- When importing a pool from JSON we handle the missing flag and fall back
to `False`
(https://github.com/apache/airflow/pull/32709/files#diff-b16c716393f690332e062e25defef0da5ac27e282553040b4b4dee223f1d169fR131)
- When upserting a pool (coming from `airflow pools set ...`) we have an
_optional_ flag for the CLI command (so the default is `False`)
(https://github.com/apache/airflow/pull/32709/files#diff-6e938311edf46fbd7341cc3f151407931322af0121acbfca9885b9f3aa0a8565R516-R518)
Is `get_current_api_client()` (`airflow.api.client`) something that is meant
to be used to interact with Airflow? If yes, then yes, I agree we should change
the signature as you said. I couldn't find much on this topic, [Public
Interface of Airflow
docs](https://airflow.apache.org/docs/apache-airflow/stable/public-airflow-interface.html)
only makes mention of the stable REST API and CLI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
