Skip to content

Conversation

@kazanzhy
Copy link
Contributor

@kazanzhy kazanzhy commented Mar 4, 2022

Some of the RDS functionality was added to moto recently.
Therefore, there are needed to upgrade version of the library

@boring-cyborg boring-cyborg bot added area:providers provider:amazon AWS/Amazon - related issues labels Mar 4, 2022
This was referenced Mar 4, 2022
@kazanzhy kazanzhy changed the title Update moto library to version 3.0 Upgrade moto library to version 3.0 Mar 4, 2022
@potiuk
Copy link
Member

potiuk commented Mar 5, 2022

This is fine - but it requires constraints to get regenerated. The failure there indicates that. I will need to manually update constraints and merge it at about the same time to accomodate !

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Mar 5, 2022
@github-actions
Copy link

github-actions bot commented Mar 5, 2022

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@uranusjr
Copy link
Member

uranusjr commented Mar 5, 2022

Is it impossible to support both 2.x and 3.0+?

@kazanzhy
Copy link
Contributor Author

kazanzhy commented Mar 5, 2022

Good question @uranusjr. I think it's not possible.
But as I understand for Airflow 2.2 and less we will have moto==2.x and for Airflow 2.3+ there will be moto==3.0.

@potiuk
Copy link
Member

potiuk commented Mar 5, 2022

But as I understand for Airflow 2.2 and less we will have moto==2.x and for Airflow 2.3+ there will be moto==3.0.

I think it does not really matter, moto is really a devel-only dependency - only used in tests, so it is just used for tests only. Once it works and constraints are updated - it will work. But I agree with @uranusjr, it might be a bit easier for us to just remove the limit (and it will get upgraded automatically). According to our newly agreed rules it should not be upper-bound (and this is even more so as this is test dependency only)

I will slightly modify it and see if it will pass (and use moto > 3) - it should.

@potiuk
Copy link
Member

potiuk commented Mar 5, 2022

UPDATE. Ah I see that we shoudl anyway have > 3.0.3 so I will just remove the ~ (and update the constraints as planned)

@kazanzhy
Copy link
Contributor Author

kazanzhy commented Mar 5, 2022

I had many problems with the "latest version" in my projects

If we are not fixing the version of moto then we should keep in mind that we can have failed tests for all PRs if let's say in version 3.1 something will be changed.

@potiuk
Copy link
Member

potiuk commented Mar 5, 2022

I had many problems with the "latest version" in my projects

If we are not fixing the version of moto then we should keep in mind that we can have failed tests for all PRs if let's say in version 3.1 something will be changed.

This is what our constraint mechanism provides. All "regular" PRs if they do not change setup.py/setup.cfg use the constraints to run the PR - i.e. latest "good" version of dependencies. Only in main builds (after merge or on schedule) or in case someone modifies the setup.py/.cfg we run "eager upgrade" of all dependencies (and if the tests succeed, those become new constraints - they are automatically pushed to our repo).

This way:

  1. we have stable set of dependencies for regular constraints

  2. we automatically upgrade the stable set with the newly released versions (with every merge and on nightly schedule) - and only when ALL tests succeed with them (this is great for security!). When we have no frequent flakes, we have 2-3 updates a day https://github.com/apache/airflow/commits/constraints-main (because we have 580 dependencies in total)

  3. when there is a breaking release we detect it (as our main and "dependecy changing" PRs start failing) - and it gives us a chance to fix them without impacting regular PRs

So - getting a failure in main when 3.1 is released with breaking change is GOOD and we are prepared to handle it - this will give us a chance to see it and fix it before most of the contributors even realise it happened.

If want to see more - my talk about it from November: https://youtu.be/_SjMdQLP30s?t=2519

@potiuk
Copy link
Member

potiuk commented Mar 5, 2022

And here is our (agreed in the community after recent discussion) approach on how we treat dependencies: https://github.com/apache/airflow#approach-to-dependencies-of-airflow

@potiuk potiuk merged commit de29eb6 into apache:main Mar 5, 2022
@potiuk
Copy link
Member

potiuk commented Mar 5, 2022

I rebuilt the constraints and merged :). All Good!

@kazanzhy
Copy link
Contributor Author

kazanzhy commented Mar 5, 2022

Thanks for the information.
That's a little bit more complex that I imagined

@potiuk
Copy link
Member

potiuk commented Mar 5, 2022

Ah yeah. With 580 deps we have ~ 20 new released dep versions every week :). Hard to keep up without the automation :)

@kazanzhy kazanzhy deleted the upgrade_moto_lib branch March 5, 2022 18:03
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Apr 11, 2022
@ephraimbuddy ephraimbuddy added changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) and removed type:misc/internal Changelog: Misc changes that should appear in change log labels Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge provider:amazon AWS/Amazon - related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants