-
Notifications
You must be signed in to change notification settings - Fork 415
Run common and dashboard tests also with newest available allowed packages for all deps #3100
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for dlt-hub-docs canceled.
|
3163148 to
d32b2ee
Compare
689daa9 to
8a1bb6d
Compare
| assert ( | ||
| coerce_value("text", "binary", HexBytes(b"binary string")) == "0x62696e61727920737472696e67" | ||
| ) | ||
| assert coerce_value("text", "binary", HexBytes(b"binary string")) in [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A newer version of hexbytes returns the hexcode without "0x". We have a couple of options:
- Keep as is, this means users will see other values in their destination after upgrading this lib
- Normalize result in normalizer so the result is the same regardless of lib version (kind of a breaking change, either for users on the old version or the new version)
- Update min version of hexbytes to the version with the new output. Also a kind of breaking change.
Imho this is probably an edge case. I think I would probably keep it as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HexBytes was used to make sure that 0x is present... so if that got removed it does not make sense to use it. My take: write our own HexBytes that alwasy prefixes and parses the prefix. it should be pretty simple to do.
for now I'd merge this ticket
rudolfix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sh-rp let's merge it. I always wanted to test of max dependencies too.
re. HexBytes: see my comment I bet @alkaline-0 can port this to dlt and remove the dependency :)
| assert ( | ||
| coerce_value("text", "binary", HexBytes(b"binary string")) == "0x62696e61727920737472696e67" | ||
| ) | ||
| assert coerce_value("text", "binary", HexBytes(b"binary string")) in [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HexBytes was used to make sure that 0x is present... so if that got removed it does not make sense to use it. My take: write our own HexBytes that alwasy prefixes and parses the prefix. it should be pretty simple to do.
for now I'd merge this ticket
Description
We have tests to test lower bounds of our allowed dependencies. We also want to see if a newer version of any or our dependencies (as allowed in pyproject.toml) breaks anything in dlt.