Fix the checksum type check #4578
Merged
Conversation
This file contains hidden or 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
2 tasks
boegel
changed the title
This was referenced Nov 8, 2024
Contributor
|
Question; this just allows the type-checking step to pass, is this still expected to fail to actually use dict+None (because it does in my test)? I assume i need another one of your PR that fixes that right? |
The `None` case was missed and due to the unrestricted `tuple` elem_type it may return valid for actually invalid entries. So restrict that beeing overly cautious so it may wrongly return invalid. But in that case the conversion function will be called which can do more elaborate verification. Add test checking for None in checksums.
…bility Not sure if that makes sense but at least for EB 4.x we need this.
Flamefire
force-pushed
the
fix-checksums-verification-5
branch
from
4e72116 to
099fc5b
Compare
2 tasks
Contributor
Author
I'm not sure which error but I guess you need #4711 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This is #4164 rebased on 5.0x
Except for one conflict (
string_type->str) it was trivial. So #4164 should still be viable for developThe None case was missed and due to the unrestricted tuple elem_type it may return valid for actually invalid entries.
So restrict that beeing overly cautious so it may wrongly return invalid.
But in that case the conversion function will be called which can do more elaborate verification.
Add test checking for None in checksums.
Also required for #4142 as the check now correctly handles a None value in the dict