Actually - it's not at all part of AIP-44 :). Airflow CLI was (consciously and deliberately) not part of it.
CLI is specifically treated as a "local" tool that is executed inside the security perimeter where direct database access is needed (i.e. it falls in the same category as 'scheduler', 'webserver`, `internal-api` component - all of which stil has direct DB access and AIP-44 does not change that. AIP-44 is specifically about Worker, Triggerrer and Dag File Processor direct DB access only. I think if we want to allow CLI to be used without direct DB access, a better solution would be to make it use the public REST API of ours, not the internal api that is supposed to be used by worker/triggerer/dag file processor. There will be different authentication and authorisation methods than the ones that airflow CLI would - potentially use. I think if we want to make airflow CLI use authorised/remote access - that needs a separate AIP - where we define actors that should be able to use the CLI, authorisation mechanisms etc. (could be the same as in the REST API, but it needs to be clarified - currently the authorisation of CLI is based exclusively on having access to the system/UNIX user that airflow is run with and where DB configuration to access DB is present). But coming back to the main subject - yes, I think that old API is experimental, and for a long time we have good replacement for it (i.e. the official REST API that has been battle-tested and is pretty feature-full), so It is a good idea to remove the old API (and CLI configuration option). It's not likely widely used, and it's also had experimental status for a very long time (since the beginning of Airflow 2.0) - so similarly as with the MSSQL support, we are free to decide to remove it and still keep our SemVer promises - it never made it on the Public Interface of Airflow https://airflow.apache.org/docs/apache-airflow/stable/public-airflow-interface.html - and since the very beginning we did not have an intention for it to be public API. So I would be very much for removing that whole part - even in Airflow 2.9. J. On Mon, Feb 19, 2024 at 11:53 AM Andrey Anshin <andrey.ans...@taragol.is> wrote: > > the underlying question is what are we going to do with direct DB access > from the cli > > Good point! > I guess it should be covered by AIP-44, so if there are no methods which > implement these actions by AIP-44 it is better to create it instead of just > moving old code. > > > > On Mon, 19 Feb 2024 at 14:35, Bolke de Bruin <bdbr...@gmail.com> wrote: > > > with regards to the api client: the intention was indeed to remove direct > > access to the DB, which I think is still a valid thing to do. In my > opinion > > the underlying question is what are we going to do with direct DB access > > from the cli. Is that something we want to keep or do we want a different > > design? Just dropping the implementation seems a bit 'lazy' to me, while > I > > understand that it hasn't been touched in a while. > > > > B. > > > > On Sun, 18 Feb 2024 at 12:17, Andrey Anshin <andrey.ans...@taragol.is> > > wrote: > > > > > Greetings guys! > > > > > > I want to start a discussion about deprecation cli configuration > options > > > > > > > > > https://airflow.apache.org/docs/apache-airflow/stable/configurations-ref.html#cli > > > as well as `airflow/api/client` in upcoming minor/feature release of > > > Airflow (2.9 or 2.10 depends on). > > > > > > This options control how end users retrieve information from the > > Database, > > > there is two kind provided classes > > `airflow.api.client.api_client.Client` - > > > direct access to DB, `airflow.api.client.json_client.Client` - access > > > through Experimental API > > > > > > However internal clients cover only this cli commands: > > > - trigger_dag: airflow dags trigger > > > - delete_dag: airflow dags delete > > > - get_pool: airflow pool get > > > - get_pools: airflow pool list and airflow pool export > > > - create_pool: airflow pool set and airflow pool import > > > - pool_delete: airflow pool delete > > > - get_lineage: doesn't use anywhere in CLI and Airflow codebase > > > > > > As a result, implementation of CLI spread across different modules / > > > subpackages and in some cases could use experimental API. Seems like it > > is > > > an abandoned implementation which comes from pre Airflow 2 and we have > to > > > support it nowadays. > > > > > > *My proposal*: > > > 1. Copy implementation from `airflow.api.client.local_client.Client` > into > > > the the code from appropriate modules into the airflow/cli/commands > > > 2. Set default value for `[cli] api_client` to None. In case if values > > set > > > in airflow.cfg (e.g. from the previous version of Airflow), raise > future > > > warning and update it to None if it set to > > > `airflow.api.client.local_client.Client` in other cases do not touch > > > 3. Set default value for `[cli] endpoint_url` to None. In case if > values > > > set in airflow.cfg raise future warning, and do not update value. > > > 4. `airflow.api.client.get_current_api_client` should able to return > > None, > > > if it return None, then it should use implementation from the > > > airflow/cli/commands, otherwise use deprecated client, with raising > > > RemovedInAirflow3Warning > > > > > > WDYT? > > > > > > > > > -- > > > > -- > > Bolke de Bruin > > bdbr...@gmail.com > > >
