Oh.. I've totally forgotten about this discussion.
We have some time before Airflow 2.9, so there is a chance that we could
remove it into 2.9.
Let me create a vote.
On Tue, 20 Feb 2024 at 23:10, Pierre Jeambrun <pierrejb...@gmail.com> wrote:
> +1, stable API should be favoured
>
> On Tue 20 Feb 2024 at 16:10, Jarek Potiuk <ja...@potiuk.com> wrote:
>
> > 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
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > > >
> > >
> >
>
