Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete experimental API #41434

Merged
merged 16 commits into from
Aug 16, 2024
Merged

Conversation

vincbeck
Copy link
Contributor


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@vincbeck vincbeck added the airflow3.0:breaking Candidates for Airflow 3.0 that contain breaking changes label Aug 13, 2024
@eladkal
Copy link
Contributor

eladkal commented Aug 13, 2024

Needs also to be removed from the docs

@vincbeck
Copy link
Contributor Author

I am unsure what to do with airflow/api/client/local_client.py and airflow/api/client/json_client.py. They both use the experimental APIs. Does someone have context on these clients?

@jedcunningham
Copy link
Member

I am unsure what to do with airflow/api/client/local_client.py and airflow/api/client/json_client.py. They both use the experimental APIs. Does someone have context on these clients?

This is actually why I gave up removing the experimental API last Friday 🙃. I think we should remove the whole concept of a client and let AIP-81 handle reimagining it.

@vincbeck
Copy link
Contributor Author

I am unsure what to do with airflow/api/client/local_client.py and airflow/api/client/json_client.py. They both use the experimental APIs. Does someone have context on these clients?

This is actually why I gave up removing the experimental API last Friday 🙃. I think we should remove the whole concept of a client and let AIP-81 handle reimagining it.

Does that mean that the current CI is using experimental API underneath?

@potiuk
Copy link
Member

potiuk commented Aug 13, 2024

This is actually why I gave up removing the experimental API last Friday 🙃. I think we should remove the whole concept of a client and let AIP-81 handle reimagining it.

No. It should not - we should just remove those IMHO. Full stop. There is no particular reason we should keep them

@jedcunningham
Copy link
Member

I am unsure what to do with airflow/api/client/local_client.py and airflow/api/client/json_client.py. They both use the experimental APIs. Does someone have context on these clients?

This is actually why I gave up removing the experimental API last Friday 🙃. I think we should remove the whole concept of a client and let AIP-81 handle reimagining it.

Does that mean that the current CI is using experimental API underneath?

Honestly, I didn't look closely at all once I saw the dependency. I'm not sure if those clients are even usable? I'm with Jarek, whack them.

@vincbeck
Copy link
Contributor Author

These clients are used by the CI today so I wont be able to remove them entirely in this PR. I think this should be done as part of the AIP-81 effort. I'll do my best to remove as much as I can though

@uranusjr
Copy link
Member

I posted #41491 to fix the news fragment check.

@vincbeck
Copy link
Contributor Author

I posted #41491 to fix the news fragment check.

Thank you!

@vincbeck
Copy link
Contributor Author

Tests are passing (besides flaky tests I'll restart) 🎉

@vincbeck
Copy link
Contributor Author

These clients are used by the CI today so I wont be able to remove them entirely in this PR. I think this should be done as part of the AIP-81 effort. I'll do my best to remove as much as I can though

I ended up deleting the client interface and configs associated to it. I had to keep the basic client implementation because it is used by the CI. It will be most likely be deleted/refactored in AIP-81 work

@vincbeck
Copy link
Contributor Author

The test [Special tests / Pydantic v1 test / All:Pydantic-V1-Postgres:12:3.8] is failing constantly without giving much information besides:

Error 1. Dumping containers: ['airflow-test-externalpython-airflow-run-cfb004eac98f', 'airflow-test-branchpythonvenv-airflow-run-4ef438000926', 
  'airflow-test-branchexternalpython-airflow-run-fc0df5a6d7ad', 'airflow-test-externalpython-postgres-1', 'airflow-test-branchpythonvenv-postgres-1', 'airflow-test-branchexternalpython-postgres-1', 
  'airflow-test-api-postgres-1'] for api.

@vincbeck
Copy link
Contributor Author

From logs:

2024-08-15T17:48:08.991440634Z  The files belonging to this database system will be owned by user "postgres".
2024-08-15T17:48:08.993916422Z  This user must also own the server process.
2024-08-15T17:48:08.993927093Z  
2024-08-15T17:48:08.993936700Z  The database cluster will be initialized with locale "en_US.utf8".
2024-08-15T17:48:08.993942531Z  The default database encoding has accordingly been set to "UTF8".
2024-08-15T17:48:08.993948022Z  The default text search configuration will be set to "english".
2024-08-15T17:48:08.993966947Z  
2024-08-15T17:48:08.993971987Z  Data page checksums are disabled.
2024-08-15T17:48:08.993976886Z  
2024-08-15T17:48:08.993981735Z  fixing permissions on existing directory /var/lib/postgresql/data ... ok
2024-08-15T17:48:08.995257685Z  creating subdirectories ... ok
2024-08-15T17:48:08.995550183Z  selecting dynamic shared memory implementation ... posix
2024-08-15T17:48:09.039424465Z  selecting default max_connections ... 100
2024-08-15T17:48:09.071701216Z  selecting default shared_buffers ... 128MB
2024-08-15T17:48:09.115526599Z  selecting default time zone ... Etc/UTC
2024-08-15T17:48:09.115535896Z  creating configuration files ... ok
2024-08-15T17:48:09.294766848Z  running bootstrap script ... ok
2024-08-15T17:48:09.798739062Z  performing post-bootstrap initialization ... ok
2024-08-15T17:48:09.885246230Z  syncing data to disk ... ok
2024-08-15T17:48:09.885277910Z  
2024-08-15T17:48:09.885283480Z  
2024-08-15T17:48:09.885288139Z  Success. You can now start the database server using:
2024-08-15T17:48:09.885292948Z  
2024-08-15T17:48:09.885297587Z      pg_ctl -D /var/lib/postgresql/data -l logfile start
2024-08-15T17:48:09.885303398Z  
2024-08-15T17:48:09.949442941Z  waiting for server to start....2024-08-15 17:48:09.949 UTC [48] LOG:  starting PostgreSQL 12.20 (Debian 12.20-1.pgdg120+1) on x86_64-pc-linux-gnu, compiled by gcc (Debian 12.2.0-14) 12.2.0, 64-bit
2024-08-15T17:48:09.950387580Z  2024-08-15 17:48:09.950 UTC [48] LOG:  listening on Unix socket "/var/run/postgresql/.s.PGSQL.5432"
2024-08-15T17:48:09.964868351Z  2024-08-15 17:48:09.964 UTC [49] LOG:  database system was shut down at 2024-08-15 17:48:09 UTC
2024-08-15T17:48:09.970375465Z  2024-08-15 17:48:09.970 UTC [48] LOG:  database system is ready to accept connections
2024-08-15T17:48:10.020225032Z   done
2024-08-15T17:48:10.020240311Z  server started
2024-08-15T17:48:10.192255016Z  CREATE DATABASE
2024-08-15T17:48:10.193187396Z  
2024-08-15T17:48:10.193412647Z  
2024-08-15T17:48:10.193532843Z  /usr/local/bin/docker-entrypoint.sh: ignoring /docker-entrypoint-initdb.d/*
2024-08-15T17:48:10.193614065Z  
2024-08-15T17:48:10.194953890Z  2024-08-15 17:48:10.194 UTC [48] LOG:  received fast shutdown request
2024-08-15T17:48:10.195528542Z  waiting for server to shut down....2024-08-15 17:48:10.195 UTC [48] LOG:  aborting any active transactions
2024-08-15T17:48:10.197630300Z  2024-08-15 17:48:10.197 UTC [48] LOG:  background worker "logical replication launcher" (PID 55) exited with exit code 1
2024-08-15T17:48:10.200565947Z  2024-08-15 17:48:10.200 UTC [50] LOG:  shutting down
2024-08-15T17:48:10.210847981Z  2024-08-15 17:48:10.210 UTC [48] LOG:  database system is shut down
2024-08-15T17:48:10.295198664Z   done
2024-08-15T17:48:10.295221868Z  server stopped
2024-08-15T17:48:10.296363270Z  
2024-08-15T17:48:10.296375713Z  PostgreSQL init process complete; ready for start up.
2024-08-15T17:48:10.296381053Z  

@vincbeck
Copy link
Contributor Author

Fix for this test in #41534

@vincbeck
Copy link
Contributor Author

All green :) Any additional reviews?

@vincbeck vincbeck merged commit 8807f73 into apache:main Aug 16, 2024
111 checks passed
@vincbeck vincbeck deleted the vincbeck/experimental_api branch August 16, 2024 17:11
Artuz37 pushed a commit to Artuz37/airflow that referenced this pull request Aug 19, 2024
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Aug 20, 2024
@kaxil kaxil added this to the Airflow 3.0.0 milestone Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
airflow3.0:breaking Candidates for Airflow 3.0 that contain breaking changes area:API Airflow's REST/HTTP API area:dev-tools area:providers area:webserver Webserver related Issues provider:fab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants