-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add pydantic support #136
base: main
Are you sure you want to change the base?
Add pydantic support #136
Conversation
39acaf8
to
dc648e3
Compare
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.
LGTM
af12768
to
6d22750
Compare
Pull Request Test Coverage Report for Build 10849026140Details
💛 - Coveralls |
6d22750
to
52f274f
Compare
I've made a major version bump because there's a breaking change in Tag handling. Specifically, the Tag constructor no longer accepts a name argument (it was always "scheduling" in the past), and the TagSetManager class has been simplified to become the Therefore, this code will no longer work: https://github.com/usegalaxy-eu/infrastructure-playbook/blob/46931230ccaa357a5420b1d4698df94221dfa168/files/galaxy/tpv/tool_defaults.yml#L39 Instead, it would need to be:
To catch these breaking changes, we should also improve the linter to support type checking, so we should probably withhold merging this till that's done. |
@@ -63,8 +63,7 @@ users: | |||
rule_helper = RuleHelper(app) | |||
# Find all destinations that support highmem | |||
destinations = [d.dest_name for d in mapper.destinations.values() | |||
if any(d.tpv_dest_tags.filter(tag_value='highmem', | |||
tag_type=[TagType.REQUIRE, TagType.PREFER, TagType.ACCEPT]))] | |||
if 'highmem' in (d.tpv_dest_tags.require + d.tpv_dest_tags.prefer + d.tpv_dest_tags.accept)] |
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.
Just flagging these for release notes. This is a simplification we can use over the existing code, but the existing filter method will also still work.
Add a well-defined schema for the TPV config with stronger typing support.