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

Add script to auto-generate PRs that update the ESGF scrape #115

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

znichollscr
Copy link
Collaborator

@znichollscr znichollscr commented Aug 21, 2024

Description

Checklist

Please confirm that this pull request has done the following:

  • Documentation added (where applicable)
  • Changelog item added to changelog/

@znichollscr
Copy link
Collaborator Author

@durack1 this script works (this PR was auto-generated with it #112). The things I don't know about are:

  1. how to automate the git operations on nimbus in a secure way. I assume we'd need to clone the repo as root or something so that only the cronjob could update it.
  2. how to store the GitHub API key securely on the server. Again, I assumed we'd have to make it only readable by root or something for actual security

In the meantime, we can obviously just run this script and make PRs by hand and it's fine.

@durack1
Copy link
Contributor

durack1 commented Aug 21, 2024

The easiest thing would be that I pull that onto the same machine (not nimbus) that is running the cronjob

@znichollscr
Copy link
Collaborator Author

The easiest thing would be that I pull that onto the same machine (not nimbus) that is running the cronjob

That'd work if you're happy with the security

git checkout -b "${BRANCH_NAME}"
git commit -m "Update ESGF input4MIPs JSON file"
git push --set-upstream origin "${BRANCH_NAME}"
gh pr create --title "Bot: ${TIMESTAMP} update ESGF JSON" --body "Created from nimbus bot" --label "esgf-json-update" --reviewer znichollscr #--reviewer durack1
Copy link
Contributor

Choose a reason for hiding this comment

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

is this gh an alias of yours?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nah it's an official github tool, see e.g. https://anaconda.org/conda-forge/gh

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Other installation options here https://github.com/cli/cli#linux--bsd

Copy link
Contributor

Choose a reason for hiding this comment

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

Sheez, yet another dependency... We're starting to drift off the KISS principle considerably..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We're starting to drift off the KISS principle considerably

I mean, yes, with a few extra thoughts.

First: this is totally optional, we can also just change

git commit -m "Update ESGF input4MIPs JSON file"
git push --set-upstream origin "${BRANCH_NAME}"
gh pr create --title "Bot: ${TIMESTAMP} update ESGF JSON" --body "Created from nimbus bot" --label "esgf-json-update" --reviewer znichollscr --reviewer durack1

to

git commit -m "Update ESGF input4MIPs JSON file @znichollscr @durack1"
git push --set-upstream origin "${BRANCH_NAME}"

and we'll still get notified I think, we just have to make the MR by hand.

Second thought: the GitHub CLI is already what Dan is using for making all the automated PRs, issues etc. so this complexity is already in our stack. Rolling our own solution would be much more headache than using what GitHub provides (which seems pretty good and stable to me). Anyway, as I said, totally optional to use the GitHub CLI here.

Copy link
Contributor

Choose a reason for hiding this comment

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

the GitHub CLI is already what Dan is using for making all the automated PRs, issues etc. so this complexity is already in our stack.

Fair comment, in order to leverage automated functionality there will be an increment in dependence that has to occur, more dependencies == less manual work (maybe). So point taken.

I'll have to figure out what is needed in an env to make this work, but let's merge with the machine/crontab configuration a second step which requires the merge in place anyway. We don't have an open issue for this do we? So if this is merge we'll have to hunt for it as a remaining to-do

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just to make sure I understand, the idea would be that the crontab job would also make the MR? If yes, makes sense. I can help with environment set up.

I'd suggest picking this back up again once we're through the CEDS/volcanic work. It could be a bit fiddly, so best to do without too many other moving parts going at the same time I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to make sure I understand, the idea would be that the crontab job would also make the MR?

If you mean Pull Request (PR), exactly, which is why the command line gh would be required, which requires the env of it's own..

I'd suggest picking this back up again once we're through the CEDS/volcanic work

100%, there will be no changes unless a publication occurs, which will happen at the very earliest next Tuesday 3 Sept (Monday is the Labor Day holiday, and I am on vacay 5-11 Sept)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you mean Pull Request (PR), exactly

Oops yes my bad.

Base automatically changed from rename-esgf-json to main August 28, 2024 15:29
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.

2 participants