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

MNT: Use '--skip-existing' over '--force' #11

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

matthewfeickert
Copy link
Member

Use anaconda-client upload command's --skip-existing flag over the --force flag to avoid redundant uploads, saving time, and to avoid uploading when legitimate errors exist. From anaconda upload --help:

...
--force: Force a package upload regardless of errors
--skip-existing: Skip errors on package batch upload if it already exists
...

* Use anaconda-client upload command's '--skip-existing' flag over the
  '--force' flag to avoid redundant uploads, saving time, and to avoid
  uploading when legitimate errors exist. From 'anaconda upload --help':
  > --force: Force a package upload regardless of errors
  > --skip-existing: Skip errors on package batch upload if it already exists

* Use long versions of command flags to make things more explicit.
@Carreau
Copy link
Collaborator

Carreau commented May 30, 2023 via email

@matthewfeickert
Copy link
Member Author

matthewfeickert commented May 30, 2023

the main reason behind --force is that some repos may not have a properly numbered wheel, and thus we actually do need to force. At least that is what I seem to remember.

@Carreau Thanks for the info, though this seems surprising to me. Why are any of the libraries targeted here not following PEP 440? If they aren't, this seems like it should be an exception to be handled with an option and not the default. Can you comment which libraries this is relevant for? Or maybe here you mean scikit-learn which overwrites the same .dev0 wheel (c.f. my note from discussion with Andreas Müller in https://discuss.scientific-python.org/t/interest-in-github-action-for-scipy-wheels-nightly-uploads-and-removals/397)? From @betatim's comment #8 (comment) it isn't clear that scikit-learn is a target. Is it? Though this shouldn't matter as the hash is different between the wheels and so --skip-existing won't skip the upload here (I think).

(No rush to respond to this, as I'm about to go offline in the US).

@Carreau
Copy link
Collaborator

Carreau commented May 30, 2023

I'm pretty sure IPython overwrite the same .dev0 as well, not because I want to, but because I never had the time to properly set it up to actually bump the dev version on nightly build and cleanup the setup.py (yet we still have a setup.py, I know, and yes I want to remove it).

@betatim
Copy link
Contributor

betatim commented May 31, 2023

I have no opinion on what this action should/shouldn't do. I think for existing projects discussing compliance with PEPs is a good way to spend a few weeks without achieving much :-/ The pragmatist in me is Ok with that.

@matthewfeickert
Copy link
Member Author

Given anaconda/anaconda-client#655 (comment) anaconda-client is uploading based off of name, so I'll have to think about if there is some cleaner way to do this. Putting this into draft for the time being, but maybe closing later if I can't think of something better.

@matthewfeickert matthewfeickert marked this pull request as draft May 31, 2023 14:58
@stefanv
Copy link
Member

stefanv commented May 31, 2023

@matthewfeickert I like the idea, in principle. Could we control this using a with clause? E.g.

with:
  overwrite: true

@Carreau
Copy link
Collaborator

Carreau commented Jun 14, 2023

I have no objections of having parameters, but at the same time I also do not want to make this a kitchen-sink utilities. At some point it should be ok to say "this is beyond the scope of this action, if you wish to have more control please use XXX". For example https://github.com/OpenAstronomy/publish-wheels-anaconda has more parameters and also take care of the removal.

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.

None yet

4 participants