Oh God . What an archeology. Yeah I think we had a limite when we discussed
it then (looking at the discussion) we did not have a solid replacement yet
(and the discussion was about deprecating it in 1.10). Since then we marked
it as experimental and left it in 2.0.
But since then - we have - and it's well established and proven (REST
API + Python Client) which very much serves the same purpose that the
"remote" experimental CLI should serve (and I think that was the main
problem that it did not exist back then).
I'd say it calls for removal and significant note ("If you were still using
it, please use the REST API and Python Client instead").
On Tue, Feb 20, 2024 at 11:19 AM Andrey Anshin <andrey.ans...@taragol.is>
wrote:
> BTW, I've found that this this top already discussed in the past, like more
> that 3 years ago
>
> Github Issue https://github.com/apache/airflow/issues/10552 (accidently
> found when set sorting on issues as Least recently updated)
> Dev List: https://lists.apache.org/thread/jdz9l7bsnsw5c3t27dxfrx5pd4wvjlxt
>
>
>
> On Tue, 20 Feb 2024 at 00:14, Andrey Anshin <andrey.ans...@taragol.is>
> wrote:
>
> > Just for clarification, the proposal is about depreciation or even
> removal
> > of something obsolete. If we could do some improvement at the same moment
> > it would be nice, if not also nice, that mean no additional work
> >
> >
> >
> >
> >
> > On Mon, 19 Feb 2024 at 22:55, Scheffler Jens (XC-AS/EAE-ADA-T)
> > <jens.scheff...@de.bosch.com.invalid> wrote:
> >
> >> I do not habe a strong opinion on the topic but I am mostly with Jarek.
> >>
> >> The API must be multi tenant and mask all internal DB access - where we
> >> are close-by with AIP-44 and the other efforts of multi tenancy
> (missing a
> >> list of all planned efforts, would be cool to have a mata AIP keeping
> the
> >> completed and planned individual efforts)
> >>
> >> But in contrast I see the CLI only partly being a tool on tenant but
> most
> >> parts on the internal perimeter admin side. Might be a bit „gray zone“.
> If
> >> we habe overlap to REST API (do we have a list of rhings like trigger
> sag
> >> or so?) we should remove and consolidate redundancy to API logic. But
> there
> >> is for sure some „black magic“ which must not be migrated and needs
> native
> >> DB access. E.g. resetting DB or triggering DB migration. For REST API as
> >> well as internal API you need a web endpoint incl. minimal
> authentication.
> >> Some base admin tooling is needed for bootstrap and base admin. Like
> >> creating the first admin user is impossible if no user.
> >>
> >> So before a lengthy discussion I‘d propose to document details and make
> >> an AIP to discuss, that might be easier then a reply-to-all thred to
> define
> >> the scope of cleanup. Also and especially for API cöient we need to
> check
> >> for impact if breaking changes appear.
> >>
> >> Jens
> >>
> >> Sent from Outlook for iOS<https://aka.ms/o0ukef>
> >> ________________________________
> >> From: Andrey Anshin <andrey.ans...@taragol.is>
> >> Sent: Monday, February 19, 2024 3:54 PM
> >> To: dev@airflow.apache.org <dev@airflow.apache.org>
> >> Subject: Re: [DISCUSS] Deprecate cli options in Airflow Configurations
> >> and airflow.api.client
> >>
> >> >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://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fairflow.apache.org%2Fdocs%2Fapache-airflow%2Fstable%2Fpublic-airflow-interface.html&data=05%7C02%7CJens.Scheffler%40de.bosch.com%7C7183c980c1634387b9c808dc315ab031%7C0ae51e1907c84e4bbb6d648ee58410f4%7C0%7C0%7C638439512771163568%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=hJSW%2FH7qQHQIH0YFHjxAoHWBNSejl2yUBpMx1Q9PQAo%3D&reserved=0
> >> <
> >>
> 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://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fairflow.apache.org%2Fdocs%2Fapache-airflow%2Fstable%2Fconfigurations-ref.html%23cli&data=05%7C02%7CJens.Scheffler%40de.bosch.com%7C7183c980c1634387b9c808dc315ab031%7C0ae51e1907c84e4bbb6d648ee58410f4%7C0%7C0%7C638439512771174117%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=dQ3MPz8rgbWo%2FIezHpvAuJ4fxDAe%2FT9hx3WRloR3ZRQ%3D&reserved=0
> >> <
> >>
> 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
> >> > > >
> >> > >
> >> >
> >>
> >
>
