Conversation
Signed-off-by: rishhavv <rishav.katoch17300@gmail.com>
|
Fixes issue number #204 |
|
Hey @rishhavv |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #210 +/- ##
==========================================
- Coverage 94.17% 94.13% -0.05%
==========================================
Files 55 55
Lines 2317 2317
Branches 119 119
==========================================
- Hits 2182 2181 -1
- Misses 98 99 +1
Partials 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| google-cloud = "^0.34.0" | ||
| google-api-core = "^2.11.0" | ||
| protobuf = ">=3.20,<5.0" | ||
| protobuf = "^5.26.1" |
There was a problem hiding this comment.
| protobuf = "^5.26.1" | |
| protobuf = ">=3.20,<6.0" |
let's not hardcode to 5.26.1, rather put some bound values? will the above work?
There was a problem hiding this comment.
@vigith this does not mean it is hard-coded. This means that it will take any version >=5.26.1 and <6.0
Nevertheless, I would recommend having a stricter constraint.
| protobuf = "^5.26.1" | |
| protobuf = "~5.26.1" |
which translates to >=5.26.1 and <5.27
There was a problem hiding this comment.
let's put >=3.20,<5.27 then? earlier we were saying we support only a lower bound of >3.20, is it not longer valid?
There was a problem hiding this comment.
Sure, but assuming that things would still work with previous versions of protobuf (<5.0) that we support.
There was a problem hiding this comment.
since protobuf follows semantic versioning, I am assuming that they won't break anything till they reach 6.0. Now the question is, is 3.x and 4.x still compatible with 5.x, if not we have to bound ourselves to strictly 5.x.
Since our tests were working with earlier version, i am gonna assume that we are good with 3.x and 4.x.
@kohlisid could you please verify these "assumptions"? 🙂
There was a problem hiding this comment.
@vigith I did test with 5.x version - everything worked fine out of the box.
I can verify once with 3.x and 4.x once.
But do we know why we had a limit in the first place?
There was a problem hiding this comment.
Also, I think it's better to set it as >=3.20,<6.0, breaking changes are not released in minor upgrades and knowing that protobuf is a library used by millions - it's even more unlikely that any breaking change will be released with minor upgrade.
There was a problem hiding this comment.
IIRC during our early version of pynumaflow, we used the 3.2 version.
There was a problem hiding this comment.
Hey @rishhavv
Were you able to verify the compatibility?
|
completed via #218 |
4 participants
Fixes #204
Update protobuf dependency to support version 5.x
Description
This PR updates the protobuf dependency in
pyproject.tomlfrom">=3.20,<5.0"to"^5.26.1". This change allows pynumaflow to leverage the latest features and improvements available in protobuf 5.x.Changes Made
pyproject.tomlfrom">=3.20,<5.0"to"^5.26.1"Benefits
Testing
Related Issue
Closes #204
Checklist
pyproject.toml