-
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
Remove --dry-run CLI option and references #326
Remove --dry-run CLI option and references #326
Conversation
Thanks, this does look like it should do the trick! Let's fix the You'll also need API tokens to test the Zenodo interactions. You should be able to create your own account on the Zenodo sandbox and get your API tokens that way - let me know if you run into hiccups there! |
I got my environment set up by adding running the following
Weirdly I then had to manually copy the For my api keys I created an account on the zenodo sandbox, went to Running Running the same command without
Not sure what I did wrong to cause that either. I'm new to basically all of these tools so apologies if the solutions are obvious! |
Hey @samleonard ! Thanks for the PR. @jdangerx meet Sam; thanks for the review! Dazhong, Sam is my friend from our youth climbing team. Seems like the Zenodo API issue is the only roadblock here. I'm not very familiar with the archiver, but here are some guesses, maybe you have thoughts Dazhong:
As a side note, we just got a grant to clarify all of the PUDL docs and this archiver repo readme is ripe for a rewrite. (cc @aesharpe ) |
Hi @samleonard ! Sorry for dropping the ball on this, and thanks @katie-lamb for bumping.
That is weird! Do you mean that you had to put the text of the YAML into the I think you're running into that 403 because the Zenodo record you're trying to update belongs to our sandbox Zenodo account, not yours. The mapping from dataset name ( eia176:
production_doi: 10.5281/zenodo.7682357
sandbox_doi: 10.5072/zenodo.3158 Corresponds to https://sandbox.zenodo.org/records/3158. If you follow that link you'll also see that the record belongs to Catalyst Cooperative. There's also some funny "redirect to latest version" stuff going on, check out their versioning docs for details. In any case, I'm curious what happens if you run with
I'm suspicious that that last step will run into issues since you had to do something funny with the Don't worry about making a bunch of spurious records on the Zenodo sandbox, that's literally what it's for and they'll wipe it if things get too bloated. Happy to get on a call if you want to talk through this more! |
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.
Thanks for taking on this issue, and for bearing with us and our out of date docs / complex testing set up for external contributors. I was able to test this locally and it worked as expected, so I'll go ahead and merge it into main!
@e-belfer @jdangerx Sorry for the delay I've had a busy week! And thanks for testing and merging! I just tried the setup again from scratch to double check that I get the same issue with the Happy to report that running with I'm also happy to jump on a call if it would be helpful for demonstrating the problem, or improving the docs, or just for fun.
|
Overview
Closes #208.
What problem does this address?
The
--dry-run
option does not adequately simulate running the archiver, so it's better to rely on the--auto-publish
option.What did you change in this PR?
I removed all references to the --dry-run option.
Testing
How did you make sure this worked? How can a reviewer verify this?
To find all references to the flag I searched the repository for "dry".
I was not able to run any tests because the dependencies currently have a conflict. This repo stipulates
"ruff>=0.3,<0.4"
but thepudl
repository recently updatedruff
to 4.1.Let me know if there are any testing steps I should take!
To-do list
Tasks