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

Allow changing the GitHub API base URL via the INSTALLER_DOWNLOAD_URL env var #199

Merged
merged 4 commits into from
Oct 31, 2024

Conversation

gaborbernat
Copy link
Contributor

@gaborbernat gaborbernat commented Oct 23, 2024

Copy link
Contributor

@mistydemeo mistydemeo left a comment

Choose a reason for hiding this comment

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

Thank you! This is looking good, and thanks so much for the tests.

Just one comment - the INSTALLER_DOWNLOAD_URL environment variable means something very different to the installer. There it's the root URL at which the installer artifacts can be found, while here it's the GitHub API root. Those two URL structures aren't usually going to be compatible, so we'll want to pick a different name here - that way the two options can be set independently of each other and won't interfere.

@mistydemeo
Copy link
Contributor

Sorry if there was a mixup in the original issue over the choice of variable name!

@gaborbernat
Copy link
Contributor Author

Thank you! This is looking good, and thanks so much for the tests.

Just one comment - the INSTALLER_DOWNLOAD_URL environment variable means something very different to the installer. There it's the root URL at which the installer artifacts can be found, while here it's the GitHub API root. Those two URL structures aren't usually going to be compatible, so we'll want to pick a different name here - that way the two options can be set independently of each other and won't interfere.

For my personal use case, that would be a disadvantage, not an advantage. Yes, these two might mean slightly different, but ultimately, from an end-user point of view, both of them are, hey, here are where the artifacts are hosted at. some artifacts are installer packages, other are github releases and api metadata are really just implementation details from an end user point of view.

So I would suggest that you reconsider that. Having to set two separate environment variables to install and update my package would cause more confusion than how much benefit it would provide in my opinion. They might be beneficial for the one maintaining the site but would be a problem for the end users.

@gaborbernat
Copy link
Contributor Author

@mistydemeo so how about a compromise of: use UPDATER_DOWNLOAD_URL if set, if not then try with INSTALLER_DOWNLOAD_URL and if that is also not set use https://api.github.com. This way if they differ user can set accordingly but if it's the same we can reuse the same env-var?

We use the INSTALLER_DOWNLOAD_URL to align up with the install script.

Signed-off-by: Bernát Gábor <[email protected]>
Signed-off-by: Bernát Gábor <[email protected]>
Signed-off-by: Bernát Gábor <[email protected]>
@axel-kah
Copy link

👍 for INSTALLER_BASE_URL not pointing to the "installer" but to be used to point it to the github (mirror) base URL.

Copy link
Contributor

@mistydemeo mistydemeo left a comment

Choose a reason for hiding this comment

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

Merging this and our followup pull request. Thank you again very much for this!

@mistydemeo mistydemeo merged commit 7507f97 into axodotdev:main Oct 31, 2024
12 checks passed
@gaborbernat
Copy link
Contributor Author

Do you have a timeline when we should expect this feature to be available for uv?

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.

Allow running uv self update behind firewalls
3 participants