cthoyt / zenodo-client

A tool for automated uploading and version management of scientific data to Zenodo
MIT License
25 stars 5 forks source link

Add options to upload / update without publish #23

Closed nuest closed 10 months ago

nuest commented 1 year ago

Closes #21

nuest commented 1 year ago

AFAICT the failing tests are due to my changes for make pydantic 2 happy, so I'll revert those.

nuest commented 1 year ago

Tests work no on my fork, no idea why they do not show up here: https://github.com/nuest/zenodo-client/actions/runs/5892513675

cthoyt commented 1 year ago

@nuest any movement forward on this?

nuest commented 11 months ago

Hi! Working on this now...

nuest commented 11 months ago

Ups, did you just merge by accident?

nuest commented 11 months ago

Never mind, must have been a glitch in my local git client.

cthoyt commented 11 months ago

@nuest I did a bit of clean up - linting, adjusting code to reduce diff, removing some minor changes that make it hard to review.

I still need to figure out how to make the environment available for tests so github actions can use my api token

codecov-commenter commented 11 months ago

Codecov Report

Merging #23 (99b720d) into main (5ee911e) will decrease coverage by 27.78%. The diff coverage is 15.09%.

@@             Coverage Diff             @@
##             main      #23       +/-   ##
===========================================
- Coverage   55.96%   28.18%   -27.78%     
===========================================
  Files           6        6               
  Lines         218      259       +41     
  Branches       38       46        +8     
===========================================
- Hits          122       73       -49     
- Misses         89      186       +97     
+ Partials        7        0        -7     
Files Changed Coverage Δ
src/zenodo_client/__init__.py 100.00% <ø> (ø)
src/zenodo_client/api.py 22.02% <15.09%> (-44.91%) :arrow_down:

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

cthoyt commented 11 months ago

@nuest I improved the way tox and GHA passes around configuration. Could you please in your fork of the repo add a secret ZENODO_SANDBOX_API_TOKEN and put a key from zenodo's sandbox? If you aren't able to get one, I can email you mine.

This will allow the tests to run in full on CI

nuest commented 11 months ago

Hi cthoyt!

Sorry for my silence and thanks for your efforts - I have actually continued to work on things myself and now we have to diverged code bases... I will take a look at the token for the CI tomorrow, not sure how to continue with my other branch, see

https://github.com/nuest/zenodo-client/tree/21-v2

nuest commented 11 months ago

I merged, think I captured all you changes appropriately, but I also made some fixes due to more extensive tests (e.g., fixing upload of files for not-yet-published depositions).

I'll ping you when I'm done with the merge, needs some manual fixes.

nuest commented 11 months ago

Tests don't run through on my fork, "Too many requests". Maybe there is something wrong with the token?

https://github.com/nuest/zenodo-client/actions/runs/6177578545/job/16768995002

Tests complete fine locally (with the same token).

cthoyt commented 11 months ago

I added some additional logging and error handling to the tests, now it's able to pick up when configuration isn't correctly configured (which seems to be the case at the moment)

cthoyt commented 11 months ago

@nuest can you please break this into two separate PRs:

  1. That changes functionality (i.e. for toggling publish=False)
  2. That adds new functionality (i.e., update_metadata())

The way I suggest doing this is just removing the update_metadata() functionality from this PR and making a separate one later.

The reason I'm asking this is there are too many changes that are hard to understand and read in the diff

nuest commented 11 months ago

I can do that. Just let me know when you are done with your edits - thanks!

cthoyt commented 11 months ago

@nuest all done, it's in your court :)

nuest commented 11 months ago

:tennis: Thanks! I'll get to it as early as possible next week.

My approach would be to make new branches from main and transferring the changes manually there - the git history of this PR is beyond my skillset to "pick" from it.

cthoyt commented 11 months ago

Easier - branch off of this branch, delete the stuff you don't need

nuest commented 10 months ago

Replaced by #28 and #29