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

[bug] Automated config update exception needed for conan config install commands to prevent catch-22 #7297

Closed
michaelmaguire opened this issue Jul 2, 2020 · 11 comments · Fixed by #7311
Assignees
Milestone

Comments

@michaelmaguire
Copy link

michaelmaguire commented Jul 2, 2020

Relates to the feature discussed in #7282

I think I just ran into a bug with this.

I had made a personal fork of our teams config to test out making this change, and used it to set up my own client with:

conan config install git@bbgithub.dev.mycorp.com:mmaguir1/conan_common_config_install.git --type git

I tested that, it worked fine, I got approval and landed the PR on our team's repo, and deleted my fork.

Today I went to perform a conan operation, which failed because the fork no longer exists.

So I thought, "no problem" I'll just re-set up my config to be from my team's repo:

C:\dev\src\PORT\pract>conan config install git@bbgithub.dev.mycorp.com:team/conan_common_config_install.git --type git
Trying to clone repo: git@bbgithub.dev.mycorp.com:mmaguir1/conan_common_config_install.git
ERROR: Can't clone repo: Command 'git -c http.sslVerify=true clone "git@bbgithub.dev.mycorp.com:mmaguir1/conan_common_config_install.git" .   ' returned non-zero exit status 128.
Cloning into '.'...
ERROR: Repository not found.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

It now won't let me fix the problem, because before the new config install location can be set, it tries to check the old, non-existent location and errors out.

Of course I worked around this, but can we please add an exception to the automated scheduler checks for conan config install commands themselves?

@memsharded
Copy link
Member

Hi @michaelmaguire

We added to Conan 1.27 two commands:

conan config install --list
conan config install --remove=index

To be able to remove configurations you don't want anymore. Would they solve the issue? What we didn't want to add is to quiet or silently pass over missing configuration origins. That should be an error and that should raise, then Conan provide the tool to manage the conan config install origins more easily.

What is not clear is why when you do conan config install <repo> it will call the other. Was it an auto-scheduled call?

@michaelmaguire
Copy link
Author

Yes, there was an auto scheduled config due.

I can't make any more conan calls without it trying to check for updates from the old, non-existent location.

@memsharded
Copy link
Member

Ok, I see what is happening.

You should at least be able to conan config install --list and then remove the conan config install --remove=index offending/non-existing origin. That will not try to launch the auto-scheduled call.

However, for cases where it fails (for example, no internet connection), it is likely that users don't want to remove the origins. So most likely, the scheduled task should always succeed in the sense that it update the timestamp, and wait for next scheduled call. What do you think @uilianries? I'd say to improve this for next release.

@michaelmaguire
Copy link
Author

michaelmaguire commented Jul 3, 2020

it is likely that users don't want to remove the origins

In this case, the command I was trying to run was to sent a new origin, which I was unable to do.

conan config install git@bbgithub.dev.mycorp.com:team/conan_common_config_install.git --type git

It seems like there should be an exception to the update check for the conan config install command.

I updated to 1.27, and it still suffers from this catch-22:

C:\dev\src>conan --version
Conan version 1.27.0

C:\dev\src>conan config install --list
Trying to clone repo: git@bbgithub.dev.mycorp.com:mmaguir1/conan_common_config_install.git
ERROR: Failed conan config install: Can't clone repo: Command 'git -c http.sslVerify=true clone "git@bbgithub.dev.mycorp.com:mmaguir1/conan_common_config_install.git" .   ' returned non-zero exit status 128.
Cloning into '.'...
ERROR: Repository not found.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

C:\dev\src>conan config install git@bbgithub.dev.mycorp.com:nx/conan_common_config_install.git --type git
Trying to clone repo: git@bbgithub.dev.mycorp.com:mmaguir1/conan_common_config_install.git
ERROR: Failed conan config install: Can't clone repo: Command 'git -c http.sslVerify=true clone "git@bbgithub.dev.mycorp.com:mmaguir1/conan_common_config_install.git" .   ' returned non-zero exit status 128.
Cloning into '.'...
ERROR: Repository not found.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

It's very easy to repro -- just fork your config repo, set config_install_interval = 1m, then conan config install it. Wait a minute and delete your fork. The only way to work around is to re-fork.

@memsharded
Copy link
Member

Thanks for the detailed report. You are right, this is a completely a bug, needs to be fixed. Please @uilianries have a look.

@michaelmaguire
Copy link
Author

No worries. It's a nice feature. You want to make an omelette, you need to break a few eggs.

@uilianries
Copy link
Member

@memsharded Not only this problem, but if there is no config listed in the index we will have a problem. I think we should put a warning, and ignore the error.

uilianries added a commit to uilianries/conan that referenced this issue Jul 3, 2020
Signed-off-by: Uilian Ries <uilianries@gmail.com>
uilianries added a commit to uilianries/conan that referenced this issue Jul 6, 2020
Signed-off-by: Uilian Ries <uilianries@gmail.com>
uilianries added a commit to uilianries/conan that referenced this issue Jul 6, 2020
Signed-off-by: Uilian Ries <uilianries@gmail.com>
uilianries added a commit to uilianries/conan that referenced this issue Jul 6, 2020
Signed-off-by: Uilian Ries <uilianries@gmail.com>
czoido pushed a commit that referenced this issue Jul 7, 2020
* #7297 fix config install when scheduled

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* #7297 Do not run sched config and empty file

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* #7297 Do not sched config with empty configs file

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* #7297 config is not only subcommands

Signed-off-by: Uilian Ries <uilianries@gmail.com>
@czoido
Copy link
Contributor

czoido commented Jul 7, 2020

Merged the fix, will be release in 1.28

@michaelmaguire
Copy link
Author

I upgraded to v 1.28.1 and can confirm this is fixed for me now. Thanks!

@memsharded
Copy link
Member

I upgraded to v 1.28.1 and can confirm this is fixed for me now. Thanks!

Great, thanks for checking it and reporting @michaelmaguire!

@michaelmaguire
Copy link
Author

Note:

Don't forget after testing this to use conan config install --list and if necessary conan config install --remove=1 or conan config init --force to clean up your config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants