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

Do not attempt to validate non-relevant files (like READMEs) #46

Closed
annakrystalli opened this issue Oct 3, 2023 · 2 comments · Fixed by #45
Closed

Do not attempt to validate non-relevant files (like READMEs) #46

annakrystalli opened this issue Oct 3, 2023 · 2 comments · Fixed by #45
Labels
bug an unexpected problem or unintended behavior CI validation-function

Comments

@annakrystalli
Copy link
Member

annakrystalli commented Oct 3, 2023

The following PR to add to the README in model-output triggered an unwanted (and unsuccessful) validation attempt on the README file. https://github.com/cdcepi/FluSight-forecast-hub/actions/runs/6391216357/job/17346023767?pr=16

We obviously want to avoid this behaviour but not 100% sure whether the best way is to:

  1. Only validate files with specific valid extensions (e.g. in model-output: csv, parquet or arrow and in model-metadata: yaml or yml. PROs: specific, CONS: Is somewhat doing an upfront extension validation/filtering. Would allow for polluting the directories with ignored invalid files
  2. Ignore md, Rmd and perhaps txt files when validating PR? PROs: will only ignore specific types of files and catch any stray invalid files. CONS: would hubs likely want to legitimately store other types of files in said folder (that would trigger a validation)? Is this something we want to discourage in any case?

Is this something you think hub admins should have some control over, i.e. we need to provide some options?

Thoughts @elray1 @nickreich or anyone else?

@nickreich
Copy link
Contributor

nickreich commented Oct 3, 2023

Another more specific option could be to just ignore any README file of any type (e.g. md, Rmd, txt, ...). I agree that we don't really want to be in the business of allowing lots of junky files in the folders, but having a README exception seems like a reasonable way to go.

I wouldn't protest implementing either of the options above either, but I think would prefer some very limited exception.

@annakrystalli
Copy link
Member Author

I like your recommendation @nickreich ! I'll go with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior CI validation-function
Projects
Development

Successfully merging a pull request may close this issue.

2 participants