-
Notifications
You must be signed in to change notification settings - Fork 24
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
Validate actions during their creation, store only configVersion in the Job schema #1341
Comments
I'm back from my vacation and just seeing this. Validation of the job config file needs to happen in the constructor, as you say. The reason for the validate() method on actions was to allow validation of None of the MVP actions used jobParams (other than the datasetIds list) so including validate() in the abstract interface was more about future-proofing the API than from an immediate need. But if we're removing validate() then we should probably remove |
@sbliven I was thinking about that validate() function when I stumbled upon this enum JobsConfigSchema. According to this enum, job wouldn't have the |
|
Based on the comment under PR #1286 and discussing afterwards with @sofyalaski:
validate()
in the action class. Validation should happen inside the constructor of each action.create
: No need for additional logic. If there is any error while constructing the actions, the application fails. When all actions have been initialized successfully, then each job that gets created will do so by automatically using the current global configuration.statusUpdate
: We need to check if theconfigVersion
is the same. If no changes were made to the configuration, then we can proceed directly to performing the update. But if the configuration has changed, would we need to update the value that is stored in the db? Note that when updating the configuration, the application needs to get restarted, so the actions have already been reconstructed automatically with the new configuration values.The text was updated successfully, but these errors were encountered: