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

#118 Read in and validate policies for universal config properties #151

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

carter-cundiff
Copy link
Contributor

  • Updated config store to make use of PropertyKey object instead of group name and property name strings
  • Updated java and python implementations of Policy to take a list of targets rather than a single target while maintaining backwards compatability

}
}

public Set<Property> getProperties() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I: Removed unused function

<BASE_URI>/tmp/configurations/base</BASE_URI>
<ENVIRONMENT_URI>/tmp/configurations/example-env</ENVIRONMENT_URI>
<BASE_PROPERTY_URI>${project.basedir}/src/test/resources/configurations/base</BASE_PROPERTY_URI>
<ENVIRONMENT_PROPERTY_URI>${project.basedir}/src/test/resources/configurations/example-env</ENVIRONMENT_PROPERTY_URI>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I: Moved unit tests to use existing policies within /src/test/resources/ rather than writing them to a /tmp directory outside of the repo

)
return self.configuredTargets[0] if len(self.configuredTargets) > 0 else None

def set_deprecated_targetConfigurations(self, new_value: ConfiguredTarget):
Copy link
Contributor

Choose a reason for hiding this comment

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

Q/A: Do we need to support the setter? I'd have thought this was a read-only property that's determined by the definition JSON file.

Copy link
Contributor Author

@carter-cundiff carter-cundiff Jun 17, 2024

Choose a reason for hiding this comment

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

In theory we probably don't since these would only be set by the abstract policy manager which does it through the new property. However the java implementation has a now deprecated setter for it, so can't hurt to maintain parity here.

@carter-cundiff carter-cundiff force-pushed the 118-universal-config-rules branch 2 times, most recently from 2a6090e to 86897b3 Compare June 17, 2024 19:19
@carter-cundiff carter-cundiff force-pushed the 118-universal-config-rules branch from 86897b3 to f807b50 Compare June 17, 2024 20:10
@carter-cundiff carter-cundiff merged commit cf5f1d3 into dev Jun 17, 2024
@carter-cundiff carter-cundiff deleted the 118-universal-config-rules branch June 17, 2024 20:11
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