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

Enhancement: TIUP_MIRRORS should be temporary runtime effected #651

Open
cyliu0 opened this issue Aug 4, 2020 · 5 comments
Open

Enhancement: TIUP_MIRRORS should be temporary runtime effected #651

cyliu0 opened this issue Aug 4, 2020 · 5 comments
Labels
status/discuss Indicates that the issue is in discussion, if no reply for long time, the issue will be closed. type/feature-request Categorizes issue as related to a new feature.

Comments

@cyliu0
Copy link

cyliu0 commented Aug 4, 2020

Feature Request

Is your feature request related to a problem? Please describe:

We can set mirror via the tiup mirror set and environment variable TIUP_MIRRORS. But the environment variable TIUP_MIRRORS also will be a permanent impact on tiup. This is not a typical way to use environment variables. When I use an environment variable, I'd assume that this is a temporary runtime setup.

We must use tiup mirror set to restore the location after we use TIUP_MIRRORS. And if I forgot to restore the mirror location, I cannot update the manifest following the official mirror anymore. This could be a little bit tricky for users. And it might cost us some OnCall work as well.

Describe the feature you'd like:

I'd like to use the TIUP_MIRRORS with runtime effected. While TIUP_MIRRORS is empty, tiup should use the default location. For different mirror locations, it should be isolated from each other. With this feature, tiup will be much easier to use and test. And there will be no such confusion like above any more.

Describe alternatives you've considered:

Teachability, Documentation, Adoption, Migration Strategy:

@cyliu0 cyliu0 added the type/feature-request Categorizes issue as related to a new feature. label Aug 4, 2020
@cyliu0
Copy link
Author

cyliu0 commented Aug 4, 2020

@lonng PTAL

@lonng
Copy link
Contributor

lonng commented Aug 4, 2020

@cyliu0 Thanks for this suggestion. It's a good idea to distinguish the environment variable and tiup mirror set subcommand. I think we can change the behavior according to your suggestion.

@lonng lonng added this to the v1.2.0 milestone Aug 4, 2020
@lucklove lucklove modified the milestones: v1.2.0, v1.3.0 Aug 31, 2020
@lucklove lucklove modified the milestones: v1.3.0, v1.4.0 Oct 12, 2020
@lucklove
Copy link
Member

@lonng I think we should deprecate TIUP_MIRRORS after we introduced tiup mirror set. It's just for compatibility. Next step we should remove this env instead of change it's behavior.

@lucklove lucklove added the status/discuss Indicates that the issue is in discussion, if no reply for long time, the issue will be closed. label Oct 19, 2020
@lucklove lucklove removed this from the v1.4.0 milestone Oct 19, 2020
@lonng
Copy link
Contributor

lonng commented Oct 19, 2020

@lucklove It's better to keep it in 1.x and remove it at 2.x.

@dveeden
Copy link
Contributor

dveeden commented Jan 4, 2022

I think keeping TIUP_MIRRORS would be good for this:

tiup mirror clone /tmp/mirror --full
tiup mirror set /tmp/mirror
tiup install ...
TIUP_MIRROR="https://tiup-mirrors.pingcap.com/" tiup mirror clone /tmp/mirror --full

Here TIUP_MIRROR should temporarily override the setting. So installs etc use the local mirror while tiup mirror clone can clone from the remote mirror. Another option would be to add something like --from="https://tiup-mirrors.pingcap.com/" to tiup mirror clone. Cloning multiple times is needed to update a cloned repository after new versions are released etc.

Cloning might be needed due to bandwidth restrictions, (partly) offline operation, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/discuss Indicates that the issue is in discussion, if no reply for long time, the issue will be closed. type/feature-request Categorizes issue as related to a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants