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

Modified zest.releaser to avoid "register" command when not publishing to public PyPi #192

Closed
wants to merge 2 commits into from

Conversation

willowmck
Copy link

The private PyPi cloud repositories often don’t use the register
command causing publish to fail. Don’t use this command unless you are
publishing to the public one.

…g to public PyPi

The private PyPi cloud repositories often don’t use the register
command causing publish to fail.  Don’t use this command unless you are
publishing to the public one.
@mauritsvanrees
Copy link
Member

Actually, on the public pypi, separately registering is not needed anymore and will actually fail with current twine.

The public pypi is also the only server where we know for sure how to ask if a package is already registered or not. But this is no longer needed, because, as said, registering is not needed.

So we need to clean this up. A proposal:

  • No longer check if a package is registered on PyPI (or anywhere else, but we were not doing that anyway).
  • Split the 'do you want to register/upload' question into two, for clarity.
  • If the repository url is https://upload.pypi.org/legacy/ (or maybe: starts with https://upload.pypi.org) don't even ask to register, as it is not needed and not supported.
  • For other repositories, do ask if the user wants to register.
  • For the default answer, we read a new register boolean config option from setup.cfg/pypirc. By default: don't register.

Does that sound sane?

It it quite different from the approach in your pull request. Would you be willing to work on this new approach? Otherwise I can see when I have the time.

@willowmck
Copy link
Author

Yes, that sounds like a good approach. I can probably get to this work later this week or early next week. Or, if you feel like doing it, that's fine too.

@willowmck
Copy link
Author

Hi, I'm thinking that the URL equivalence check is not necessary if we simply turn off register by default and make it a configuration option. Does that approach sound okay to you?

1. Changed register action to False by default.
2. Added a config boolean to retrieve the register action override.
3. Refactored the logic to retrieve a boolean value from the config
file to reduce code duplication.
4. Simplified the logic in upload_distributions to only call register
if the config file sets register to yes.
return default
return self.__get_boolean('zest.releaser', 'no-input')

def __get_boolean(self, section, key):
Copy link
Member

Choose a reason for hiding this comment

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

The __get_boolean method should accept a default value. Currently you make the default False. In want_release we want True as default, which is not happening now.

Also, I would rename this to _get_boolean or _getboolean. A double underscore indicates a special method, which is not the case here.

do_register = False
question = "Upload"
if self.pypiconfig.register_package():
logger.info("Registering...")
Copy link
Member

Choose a reason for hiding this comment

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

If a user has multiple servers in his ~.pypirc, this change means we register at all three servers, without asking any questions.
I would prefer this:

  • Ask if user wants to upload to server X.
  • No: don't upload and don't register.
  • Yes: if register is on in the config, we register. And then we upload.

@mauritsvanrees
Copy link
Member

I have added a few remarks in the commit. For the rest it looks good.
Well, Travis complains. :-) And in the doctests it can be very tricky to see what the exact problem is. :-/

@mauritsvanrees
Copy link
Member

This was replaced by your own new pull request #194, right? I will close this one then.

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

2 participants