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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add airflow 2.0 min requirements #395

Merged
merged 2 commits into from Dec 7, 2020
Merged

Conversation

adam2k
Copy link
Contributor

@adam2k adam2k commented Dec 7, 2020

Description

Make sure that an Airflow deployment is using version 1.10.14 before allowing the upgrade to 2.0.x

馃師 Issue(s)

Resolves astronomer/issues#2289

馃И Functional Testing

It's possible to test this change locally.

  1. Make build with a valid version number in the Makefile
  2. Start up Houston API and Astro UI
  3. Create a new deployment with a version prior to 1.10.14. (I was testing with 1.10.12)
  4. Make sure you have Airflow 2.0 added into your database in the AirflowRelease table. If not, manually add it (you can copy and existing deployment and change the version number for testing purposes)
  5. Attempt to upgrade the airflow deployment to 2.0 (see the screen shots below).
  6. Make sure you see an error message.
  7. After getting the error message, update your deployment Airflow version to 1.10.14 and attempt to upgrade again. You should now see the success message.

馃摳 Screenshots

Screen Shot 2020-12-07 at 12 16 01 PM
Screen Shot 2020-12-07 at 12 16 23 PM

馃搵 Checklist

  • Rebased from the main (or release if patching) branch (before testing)
  • Ran make test before taking out of draft
  • Added/updated applicable tests
  • Tested against Houston-API and Astronomer (if necessary).
  • Communicated to/tagged owners of respective clients potentially impacted by these changes.
  • Updated any related documentation

@codecov
Copy link

codecov bot commented Dec 7, 2020

Codecov Report

Merging #395 (9340512) into main (d42cfb7) will increase coverage by 0.00%.
The diff coverage is 59.25%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #395   +/-   ##
=======================================
  Coverage   54.51%   54.51%           
=======================================
  Files          32       32           
  Lines        2313     2335   +22     
=======================================
+ Hits         1261     1273   +12     
- Misses        963      968    +5     
- Partials       89       94    +5     
Impacted Files Coverage 螖
deployment/deployment.go 80.82% <59.25%> (-3.39%) 猬囷笍

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 d42cfb7...9340512. Read the comment docs.

@@ -32,7 +32,7 @@ var (
settings Config

// Version 2.0.0
newAirflowVersion uint64 = 2
NewAirflowVersion uint64 = 2
Copy link
Contributor

Choose a reason for hiding this comment

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

New - is slightly confuse me, cause it's de facto constructor of objects in golang.
can we rename to something more readable rather than just NewAirflowVersion?
AirflowVersionSecond?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I didn't know what we should call it because it's a special case. AirflowVersionSecond or AirflowVersionTwo?

Copy link
Contributor

Choose a reason for hiding this comment

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

yep but i. prefer to name explain value, since it's airflow 2.0.0 specific.
AirflowVersionTwo +1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to AirflowVersionTwo

@adam2k adam2k merged commit 164f0c0 into main Dec 7, 2020
adam2k pushed a commit that referenced this pull request Dec 9, 2020
* Add airflow 2.0 min requirements

* Fix AirflowVersionTwo variable name
@ashb ashb deleted the feature/airflow-min-requirements branch December 9, 2021 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants