-
Notifications
You must be signed in to change notification settings - Fork 324
Add '--lenient' option to reduce some errors to warnings #1270
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
Conversation
At the moment this is just for `UnsupportedConfiguration`, but I imagine there may be other errors that could be swallowed in relative safety (for example, maybe any 400-class HTTP errors) if the user opts-in to it.
Hi @zware, thanks for opening a PR! I'm curious what @sigmavirus24 thinks, but I think I'm personally a soft -1 on adding this for a few reasons:
(I have some other concerns as well, but I think those are the main ones.) |
One thing I'll say is that twine today is theoretically pluggable in some ways. As it stands, we have an unadvertised entry-point that someone could write a sub-command to add additional commands to twine with. Honestly, if we had our own config file and not the inherited mess from distutils/setuptools I'd be inclined to say "Instead of supporting with best-effort hacks, folks should write a plugin to match that index's behaviour." If that were more easily configurable, I'd be inclined to allow that. Especially as it might help to make the authentication bit more pluggable too. I don't think CLI flags are the solution here, they're the quickest hack that gets us back into a hole. It's quick hacks like we initially accepted that got us here. It's the years of trying to fix those as the 3rd parties broke that compatibility with increasing fervor and frequency and laid the burden at our feet. In short, I think the best way to give people a way out of the feature removal in 6.2.0 is to:
2 is the real blocker here. And no, "Just Detect When To Use a Plugin" is a security nightmare. Folks must explicitly opt-in. And a CLI flag is one way to do this, but we must be able to read it from a config file. |
To give some context here, I was surprised to find this morning that my use case was broken1, even though I had already looked at the change to remove the I can definitely understand the reticence towards this approach, though. It definitely has a hacky feel to it, and I can see not wanting to open the door to making other things "lenient". What about a different approach, entirely separate from Footnotes |
This is my only concern as well. I fully support the removal of hacks. And as an OSS maintainer I can appreciate the extra workload this would add to maintainers, even if it's just closing issues/PRs with a prepared statement. On other hand, if the goal of the project is to provide support for repositories that behave like PyPI, then maybe a section on the docs page that clearly calls out the issue would be furthering this goal even more. You could just link to it when closing any issues/PRs. If the page lays out the integration requirements, it might also be an easy way to make user go and complain to their commercial providers. But I'm also supportive of this kind of functionality living in user-land, especially long-term. On the |
Closing in favor of #1271. |
At the moment this is just for
UnsupportedConfiguration
, but I imagine there may be other errors that could be swallowed in relative safety (for example, maybe any 400-class HTTP errors) if the user opts-in to it.This is an attempt to find a middle ground between the 6.1.0 and 6.2.0 behavior for repositories that are not exactly PyPI but actually try to be close. This allows a user to explicitly say "I know I'm on shaky ground and am my own" while allowing
twine
maintainers to say "we tried to tell you" :)I'm not particularly wedded to any of the names/strings I used; suggestions for something better (or stronger, like
--unsupported
rather than--lenient
) are more than welcome, and I'm happy to adjust to taste.