>Actually - it's not at all part of AIP-44 :). Airflow CLI was (consciously and deliberately) not part of it.
Then better to check whether or not it accidentally uses it, add to my checklist > So I would be very much for removing that whole part - even in Airflow 2.9. +1 for removal if it won't break our promises On Mon, 19 Feb 2024 at 16:01, Jarek Potiuk <ja...@potiuk.com> wrote: > 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 > > > > > >
