More defensive access of extra props in filesources #17445
Merged
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
|
That doesn't look right, FileProps probably needs some work. |
mvdbeek
reviewed
Feb 9, 2024
lib/galaxy/files/sources/__init__.py
Outdated
|
|
||
| # Property overrides for values initially configured through the constructor. For example | ||
| # the HTTPFilesSource passes in additional http_headers through these properties, which | ||
| # are merged with constructor defined http_headers. The interpretation of these properties | ||
| # are filesystem specific. | ||
| extra_props: Optional[FilesSourceProperties] | ||
| extra_props: Optional[FilesSourceProperties] = {} |
There was a problem hiding this comment.
I think this is fine if you make the class a dataclass ... which may have been the intention from the start ?
Otherwise it's error prone to have this be a class var.
mvdbeek
reviewed
Feb 9, 2024
|
You can use |
mvdbeek
approved these changes
Feb 9, 2024
|
Cool thanks! Should we target a release branch (23.1?) since it is a fix? |
nuwang
force-pushed
the
make_extra_props_more_defensive
branch
from
c54f5bb to
6a3575d
Compare
nuwang
force-pushed
the
make_extra_props_more_defensive
branch
from
6a3575d to
a644028
Compare
|
I've retargeted 23.1, but it looks like the writable property was introduced in 23.2. Should I target that instead? |
|
I think this is fine as a fix |
nuwang
force-pushed
the
make_extra_props_more_defensive
branch
from
a644028 to
4a47a43
Compare
Co-authored-by: Marius van den Beek <[email protected]>
nuwang
force-pushed
the
make_extra_props_more_defensive
branch
from
4a47a43 to
c7a5e4f
Compare
|
This PR was merged without a "kind/" label, please correct. |
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.
Based on bug and solution described in: #17442
How to test the changes?
(Select all options that apply)
License