Skip to content

Bump moto-ext to 5.0.17.post2 #11710

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 22, 2024
Merged

Bump moto-ext to 5.0.17.post2 #11710

merged 1 commit into from
Oct 22, 2024

Conversation

viren-nadkarni
Copy link
Member

@viren-nadkarni viren-nadkarni commented Oct 18, 2024 •
edited
Loading

@viren-nadkarni viren-nadkarni self-assigned this Oct 18, 2024
@viren-nadkarni viren-nadkarni added the semver: patch Non-breaking changes which can be included in patch releases label Oct 18, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 18, 2024 •
edited
Loading

S3 Image Test Results (AMD64 / ARM64)

  2 files    2 suites   3m 43s ⏱️
423 tests 369 ✅  54 💤 0 ❌
846 runs  738 ✅ 108 💤 0 ❌

Results for commit c8b2f39.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 18, 2024 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 42m 11s ⏱️ -13s
3 509 tests ±0  3 096 ✅ ±0  413 💤 ±0  0 ❌ ±0 
3 511 runs  ±0  3 096 ✅ ±0  415 💤 ±0  0 ❌ ±0 

Results for commit c8b2f39. ± Comparison against base commit 4a775c4.

♻️ This comment has been updated with latest results.

@viren-nadkarni viren-nadkarni marked this pull request as ready for review October 18, 2024 08:51
@viren-nadkarni
Copy link
Member Author

@alexrashed I guess now the only way to test this bump in Ext is to merge this first, am I correct?

alexrashed
alexrashed approved these changes Oct 21, 2024
View reviewed changes
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

The changes here are looking great, it seems the upgrade does not introduce any integration issues here in Community. 🥳

As soon as we can guarantee the compatibility in our downstream projects, we can move forward with merging this PR.

So to answer your question @viren-nadkarni: No, I think we should minimize the friction in our downstream projects as good as possible, so we should make sure they are compatible before merging this PR. This is why we should not merge this PR just now.

We can easily test compatibility with our companion branch detection. So you can just create a companion PR in our downstream repo which also bumps moto-ext. We should then check in the logs that we really are installing the right version. If this PR, testing against this branch here in community is green, we can be sure that we already have forward-compatibility in our downstream project.

@viren-nadkarni
Copy link
Member Author

@alexrashed I'm still not quite sure about this.

As I understand, pip-compile only checks the package versions from PyPI. I can run make upgrade-pinned-dependencies in this repo which will pick up the published moto-ext just fine. However, running it in ext will have no effect, because there, moto-ext is an indirect dependency via localstack-core and the latter is not yet published.

Also (correct me if I'm wrong) the 'Upgrade pinned Python dependencies' action in Ext does not have companion branch detection. But it wouldn't be useful anyway due to pip-compile's dependence on published packages.

I think one solution is to add maybe add moto-ext as a temporary dependency to localstack-ext, just verify the CI, the rely on 'Upgrade pinned Python dependencies' after community is bumped.

@dfangl dfangl mentioned this pull request Oct 21, 2024
Merged
@alexrashed
Copy link
Member

Right, that's true, that makes it a bit more complicated. However, you can just do that on your own instead of using the make target, right? So you could just modify the requirements files in a branch (with the same name) in the downstream repo, right?

@viren-nadkarni
Copy link
Member Author

viren-nadkarni commented Oct 22, 2024 •
edited
Loading

Ext suite verified passing (with the exception of Cloud Pods and Persistence tests which are known to be timing out and failing) by using a companion branch where moto-ext is manually bumped.

See AWS / Ext Integration Tests # 1550

Merging PR

@viren-nadkarni viren-nadkarni merged commit 3a54f21 into master Oct 22, 2024
43 checks passed
@viren-nadkarni viren-nadkarni deleted the bump-moto-ext branch October 22, 2024 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexrashed alexrashed alexrashed approved these changes

Assignees

@viren-nadkarni viren-nadkarni

Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@viren-nadkarni @alexrashed