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

Proper version is displayed when running prod image via Breeze #8229

Merged

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Apr 9, 2020


Make sure to mark the boxes below before creating PR: [x]


In case of fundamental code change, 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 UPDATING.md.
Read the Pull Request Guidelines for more information.

@potiuk
Copy link
Member Author

potiuk commented Apr 9, 2020

Note that this is only applicable to production image. --install-airflow-version in CI works slightly differently. In production image it builds a new image. In CI image it installs airflow version in the original "2.0" image (uninstalls airflow 2.0 first). This is much faster and OK for testing. For production image - we really want to build the image in an optimal way so the image is rebuild using the version of airflow provided via --install-airflow-version flag.

@codecov-io
Copy link

Codecov Report

Merging #8229 into master will decrease coverage by 0.43%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8229      +/-   ##
==========================================
- Coverage   88.35%   87.92%   -0.44%     
==========================================
  Files         936      936              
  Lines       45305    46065     +760     
==========================================
+ Hits        40030    40503     +473     
- Misses       5275     5562     +287     
Impacted Files Coverage Δ
...flow/providers/apache/cassandra/hooks/cassandra.py 21.25% <0.00%> (-72.50%) ⬇️
...w/providers/apache/hive/operators/mysql_to_hive.py 35.84% <0.00%> (-64.16%) ⬇️
airflow/providers/postgres/operators/postgres.py 47.82% <0.00%> (-52.18%) ⬇️
airflow/providers/redis/operators/redis_publish.py 50.00% <0.00%> (-50.00%) ⬇️
airflow/providers/mongo/sensors/mongo.py 53.33% <0.00%> (-46.67%) ⬇️
airflow/providers/mysql/operators/mysql.py 55.00% <0.00%> (-45.00%) ⬇️
airflow/providers/redis/sensors/redis_key.py 61.53% <0.00%> (-38.47%) ⬇️
airflow/executors/celery_executor.py 50.67% <0.00%> (-37.84%) ⬇️
...roviders/google/cloud/operators/postgres_to_gcs.py 54.28% <0.00%> (-31.43%) ⬇️
airflow/providers/postgres/hooks/postgres.py 78.87% <0.00%> (-15.50%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad42c40...2a6a7b5. Read the comment docs.

@potiuk
Copy link
Member Author

potiuk commented Apr 10, 2020

Hey @feluelle - can you double check if this works :) ?

Copy link
Member

@feluelle feluelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works. Thanks @potiuk :)

@feluelle feluelle merged commit 82a8985 into apache:master Apr 11, 2020
@feluelle
Copy link
Member

feluelle commented Apr 14, 2020

@potiuk That is really weird -> $AIRFLOW_VERSION shows 1.10.10 also airflow version shows 1.10.10 but endpoint /version shows 2.0.0

  1. ./breeze build-image --production-image --install-airflow-version=1.10.10
  2. ./breeze --production-image
  3. airflow upgradedb
  4. airflow webserver
  5. http://0.0.0.0:28080/version

EDIT:

being on git branch #8228 (and rebased on latest master)

@potiuk
Copy link
Member Author

potiuk commented Apr 14, 2020

it is :). Looking at it

@potiuk
Copy link
Member Author

potiuk commented Apr 14, 2020

Can you try the same with --skip-mounting-local-sources switch? I think this might be the reason and we might want to simply not mount "airflow" sources at all (the change I just approved and merged today).

@feluelle
Copy link
Member

with --skip-mounting-local-sources I cannot reach the webserver probably because the compose file (local-prod) won't be used at all.

When I remove all volumes in the compose and run normally - it works :) /version shows 1.10.10

@potiuk
Copy link
Member Author

potiuk commented Apr 14, 2020

YEah. I think the reason was source mapping to inside the container. I left comment in #8228 to remove all the /opt/airflow mounts. This might be super-misleading if we leave them.

@potiuk
Copy link
Member Author

potiuk commented Apr 14, 2020

Luckily I have not yet merged it so you can still fix it :)

@feluelle
Copy link
Member

Will do - thanks for helping :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants