fix: Loosen deps ranges for compat with api, worker #432
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #432 +/- ##
==========================================
- Coverage 90.56% 89.81% -0.75%
==========================================
Files 390 324 -66
Lines 11768 9379 -2389
Branches 1974 1677 -297
==========================================
- Hits 10658 8424 -2234
+ Misses 1026 891 -135
+ Partials 84 64 -20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
suejung-sentry
changed the title
|
NOTE here are the errors I'm seeing: APISame Workerwhen spinning up the docker container when running pytest |
Swatinem
reviewed
Nov 22, 2024
| @@ -1385,15 +1385,15 @@ wheels = [ | |||
|
|
|||
| [[package]] | |||
| name = "sentry-sdk" | |||
| version = "2.18.0" | |||
| version = "2.13.0" | |||
There was a problem hiding this comment.
this is the only package thats actually being downgraded here, and I would suggest that you not pin this, but rather define a lower bound, like for the others.
The other packages are not actually being downgraded in the lockfile, but rather their minimum version is being lowered, which has an effect on api/worker pulling in shared and having to resolve dependency versions. So in api/worker, we could end up with lower dependencies.
If you are hitting actual errors with deprecated/removed API, you can pin this to a lower version using abs<x.y.z syntax like for sqlalchemy in the pyproject.toml.
Not quite sure whether you want to do that here in this repo, or in api/worker rather.
There was a problem hiding this comment.
Good points - thanks!
Fixed the language used in the PR to be more correct lol, also Michelle helped make that a range instead of pin.
I'll bump/fix the breaking change in sentry-sdk 2.18.0 after we get to a clean slate again - codecov/engineering-team#2994
RE: the other errors, looks like they are okay in staging per here so I probably had something wrong in my setup. But think we're good to go now 🚀
|
I tested both out by migrating them and then running them on staging (uh, tested with "critical path" workflows): codecov/codecov-api#998 Both work fine. Once this change is merged, I would do the updates to both repos straight away to unblock people. |
suejung-sentry
changed the title
michelletran-codecov
approved these changes
Nov 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Loosen deps version ranges to be compatible with that currently in
apiandworker. Sets deps versions here in shared asminimum(api, worker)versionapi requirements.txt
worker requirements.txt
Follows up to https://github.com/codecov/shared/pull/423/files
Tested by spinning up api and worker using this version of shared with
docker compose up& checking for any build errors. Also ranpytestwithin those containers for any build errors there