Skip to content
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

ServiceVariables: do not store service schema #527

Closed
wants to merge 1 commit into from

Conversation

PaulFarault
Copy link
Contributor

Which issue(s) this PR fixes

Fixes None

There is no need to store the schema in the ServiceVariables class. It can be retrieve from the collections object and passed as an argument when needed.

Agreements

@PaulFarault PaulFarault self-assigned this Nov 24, 2023
@PaulFarault PaulFarault force-pushed the service-variables-schema branch from 0d9ea44 to 9c58e83 Compare November 24, 2023 14:34
Copy link
Contributor

@mdrutel mdrutel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested OK

@@ -242,7 +236,7 @@ def validate_schema(self, variables: Variables, schema: dict) -> None:
except exceptions.ValidationError as e:
raise InvalidSchema("Schema is invalid", variables.name) from e

def validate(self) -> None:
def validate(self, schema: dict) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this PR. Now when I have an instance of ServiceVariables I need to give a dict representing the schema every time I want to validate the variables? But the schema doesn't change between each validation.

Copy link
Contributor Author

@PaulFarault PaulFarault Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With #320 in mind (preventing default variables copy), we'll need to have access to default variables in the validate method. Both default variables and schema come from the Collections object, hence, it'll feel better for me to pass a collections object as an argument instead of storing everything in the ServiceVariables object (as it is only required for schema validation).

I did this PR to prepare this change.

@PaulFarault PaulFarault force-pushed the service-variables-schema branch from 9c58e83 to d426409 Compare January 16, 2024 08:57
@PaulFarault PaulFarault deleted the service-variables-schema branch February 22, 2024 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants