hubverse-org / hubValidations

Testing framework for hubverse hub validations
https://hubverse-org.github.io/hubValidations/
Other
1 stars 4 forks source link

Decide how to include meaningful yet automated config checks #35

Closed annakrystalli closed 1 year ago

annakrystalli commented 1 year ago

A lot of validation checks in hubValidations depend on a properly configured hub from which configurations can be read and submissions validated against.

One option is to validate config as part of each submission:

  1. within validate_submission(): means that it will also be checked as part of local submission checks by teams. Validation could be outdated if any changes to local config files have not been synched with upstream repo prior to submission.
  2. within validate_pr(): not run as part of local checks but run as part of PRs.
  3. A separate function run as part of the CI workflow prior to running validate_pr()

It feels like it would be useful to be able to run such validations separately from submissions at other times, e.g. at hub setup and perhaps on a cron job?

We might also want changes to config files to trigger validation checks that all model-output or model-metadata files are still conforming to the new config files?

Implementation

A hubValidations unit check function (e.g. check_config_hub_valid()) can just wrap hubUtils::validate_hub_config() function for check, return hub_check check_error object if any FALSE and return wrapper validation fn early/stop execution of further validation fns.

annakrystalli commented 1 year ago

Implemented in both validate_submission() and validate_pr(). In validate_pr() check performed as standard once and skipped during execution of validate_submission(). In validate_submission(), check performed by default but can be skipped via argument skip_check_config.

Config check is NOT implemented in lower level validation function validate_model_metadata(), validate_model_file, validate_model_data() or validate_model_submission_time() but could be in a similar fashion to how it is in validate_submission() (i.e. with the ability to be skipped).

Let me know if you think I should implement in lower level functions also.

elray1 commented 1 year ago

I think this seems like a fine answer :)