asdf-community / asdf-poetry

Poetry plugin for the asdf version manager
https://github.com/asdf-vm/asdf
MIT License
74 stars 15 forks source link

`ASDF_POETRY_INSTALL_URL`: Optionally override installer URL #16

Closed tony closed 2 years ago

tony commented 2 years ago

Escape hatch for #14 issue (python 3.10 support for >=1.1.9;<1.2) while preserving the get-poetry.py default behavior. This only changes behavior when ASDF_POETRY_INSTALL_URL is passed in. Example:

If you haven't, uninstall: asdf uninstall poetry 1.1.8;

See also: asdf-python's environmental variable e.g. ASDF_PYTHON_PATCH_URL, asdf-nodejs's environmental variable e.g. NODEJS_CHECK_SIGNATURES

Default installer for <1.2 (get-poetry.py):

asdf install poetry 1.1.8

Uninstall again:

asdf uninstall poetry 1.1.8;

Notice the warning:

This installer is deprecated. Poetry versions installed using this script will not be able to use 'self update' command to upgrade to 1.2.0a1 or later.

Force install-poetry.py:

ASDF_POETRY_INSTALL_URL=https://install.python-poetry.org asdf install poetry 1.1.8
tony commented 2 years ago

@crflynn @Kurt-von-Laven: Thoughts on this approach in favor of #14?

All existing behavior is the same. If ASDF_POETRY_INSTALL_URL is set the installer can be overridden.

Kurt-von-Laven commented 2 years ago

I do like this better than a --classic flag. The way you have specified the environment variable is certainly very flexible, but users may prefer the convenience of a boolean environment variable to save keystrokes since the Poetry install URL is intended to be stable for the foreseeable future. I defer to someone with more asdf experience in any case since I have no idea whether there is an existing convention in the ecosystem for these sorts of scenarios.

crflynn commented 2 years ago

This looks OK although I tend to agree with Kurt on preferring a boolean over a URL as I don't think there would be other options for the URL. I don't plan on using this anyway so in that regard I'm indifferent.

I'll just note that if newer versions of poetry revert to the original method of detecting python versions (and have better compatibility in general) I will re-evaluate and likely remove this feature, since I consider it a footgun as it allows for broken installations of poetry with respect to asdf functionality via #10.

tony commented 2 years ago

@crflynn Have a preference on the variable name?

crflynn commented 2 years ago

The current implementation is fine I suppose. Could you please document this under #usage in the readme?

tony commented 2 years ago

@crflynn To clarify one thing:

The current implementation is fine I suppose.

By that, does that mean keep it as-is (where install URL is specified) or keep the variable name the same and switch it to a boolean?

Could you please document this under #usage in the readme? Noted

crflynn commented 2 years ago

Lets just keep it as-is.

tony commented 2 years ago

@crflynn How is this? e036b92 (markdown, view)

tony commented 2 years ago

@crflynn Thank you for the review! @Kurt-von-Laven thank you for checking into both this and #14 as well

Kurt-von-Laven commented 2 years ago

Thanks for actually doing the work and coming up with a reasonable workaround to a sticky situation. I'm sure everyone will be looking forward to the next Poetry 1.2 release candidate.