-
Notifications
You must be signed in to change notification settings - Fork 2
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
base: main
Are you sure you want to change the base?
Conversation
@durack1 this script works (this PR was auto-generated with it #112). The things I don't know about are:
In the meantime, we can obviously just run this script and make PRs by hand and it's fine. |
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 |
scripts/check-esgf-scrape.sh
Outdated
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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..
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
Description
Checklist
Please confirm that this pull request has done the following:
changelog/