e-mission / nrel-openpath-deploy-configs

Configurations for current OpenPATH deployments, published for transparency
BSD 3-Clause "New" or "Revised" License
2 stars 11 forks source link

Updates to Config Generation #58

Closed Abby-Wheelis closed 8 months ago

Abby-Wheelis commented 8 months ago

In testing before our demo today, I realized that the workflow was still committing node-modules to the PR branch.

I recognized that this was happening in the pull-request-generation step of the workflow, and looked into the documentation for that process.

I needed to specify an 'add-path' parameter to restrict the tracked changes to the configs folder.

I plan to go back and see if there are any other review comments from previous iterations that I can also address here.

Abby-Wheelis commented 8 months ago

These two screenshots show that, when running on my main branch, only one file is committed, and that there are no node-modules files on my branch, which the accidental presence of those files previously was what prevented me from noticing that all files were being committed, since they were already on my branch.

Screenshot 2024-01-25 at 11 16 39 AM Screenshot 2024-01-25 at 11 17 09 AM

Abby-Wheelis commented 8 months ago

Before I forget what else I want to do here:

With the list size limitations, it would be ideal if I could do this as a constraint of the form, but I'm not sure if that's supported. I can absolutely document in the form that there's a limit, but if I constrain it in process that would lead to a process failure and no created pull request ... is that something we're ok with?

Abby-Wheelis commented 8 months ago

Sucessfully shortened the list to a max of 4 emails (but we could change this) - an error message will show in the action tab if it fails, I still need to add a warning to the form itself

Screenshot 2024-01-26 at 11 08 09 AM
Abby-Wheelis commented 8 months ago

Testing done: 1 form with too many emails, job failed with error message. 1 form with 3 emails, extra leading and trailing whitespace verified as gone.

Screenshot 2024-01-26 at 11 18 44 AM Screenshot 2024-01-26 at 11 18 55 AM

All pull requests only comitting the config file, verified by ensuring no node_modules files in repo before running the job, and only one file (config file) added to generated pull request

Abby-Wheelis commented 8 months ago

I have now tested the lowercase functionality! Screenshot 2024-02-01 at 5 14 39 PM

Screenshot 2024-02-01 at 5 14 29 PM

shankari commented 8 months ago

At this point, you are auto-correcting errors. I am merging this now, but think that a better approach might be to fail the process, flag the errors and let them fix them.

I wonder if we should also just create a website instead of the issue form, but I am not sure of the best way to embed the GitHub token so that we can create the PR. @nataliejschultz I know you have used AWS secrets in a website; would that be an option to store a GitHub access key?

nataliejschultz commented 8 months ago

I wonder if we should also just create a website instead of the issue form, but I am not sure of the best way to embed the GitHub token so that we can create the PR. @nataliejschultz I know you have used AWS secrets in a website; would that be an option to store a GitHub access key?

Yes, you can store the token in secrets manager. However, does the token change frequently? If so, there needs to be an automated process to update the secret with the new token. It also depends on where the website is hosted. We need to be able to assume an IAM role that has access to our token secret in Secrets Manager to retrieve the token and use it to create a PR.