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

Try connecting to Ubuntu keyserver on port 80 if default HKP port fails #722

Merged
merged 3 commits into from
Oct 17, 2023

Conversation

hoechenberger
Copy link
Contributor

@hoechenberger hoechenberger commented Oct 10, 2023

Closes #719
Closes #717

cc @samruddhikhandale

@hoechenberger

This comment was marked as outdated.

@hoechenberger hoechenberger marked this pull request as ready for review October 11, 2023 07:53
@hoechenberger hoechenberger requested a review from a team as a code owner October 11, 2023 07:53
@hoechenberger
Copy link
Contributor Author

This is ready for review.

Copy link
Member

@samruddhikhandale samruddhikhandale left a comment

Choose a reason for hiding this comment

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

@hoechenberger did this change work with your local config/machine? The tests are passing, so should be safe to merge it in. Also, I'll shortly test these changes on my local machine.

Left one minor comment.

src/git-lfs/install.sh Show resolved Hide resolved
@samruddhikhandale
Copy link
Member

@hoechenberger did this change work with your local config/machine?

I saw your PR description update, looks like this PR changes are working for you? 🎉

@hoechenberger
Copy link
Contributor Author

hoechenberger commented Oct 12, 2023

@samruddhikhandale I was struggling to get the tests to work on my corporate machine because I need to inject custom SSL certificates. I have written a custom feature to do that, but didn't have time to modify the tests to actually use this feature. So my honest answer is: no, I could not fully run the test suite yet. LMK if you would still like to go ahead and merge (after which I'd be able to test in production) and we'll have to revert if necessary; or otherwise I'll have to get the test suite to run locally, but it may take a couple of days (as I'm very busy today and probably also tomorrow)

@samruddhikhandale
Copy link
Member

@hoechenberger I have a simpler way to test these changes, can you check if you can create a dev container with https://github.com/samruddhikhandale/test-gpg-keyserver/tree/main ?

It adds the same dev configuration as mentioned in #717, the only difference is that it doesn't pull the python Feature from ghcr, instead it uses a local Feature (which has these ^ PR changes)

It would be great to know if this changes fixes your issue before we roll out this change. It works good with my testing, however, the gpg issue behaves differently on different machines (don't want to accidentally cause Feature failure blips - given that this PR updates seven Features)

@hoechenberger
Copy link
Contributor Author

@samruddhikhandale Thank you! I will test this on my macOS machine and ask colleagues of mine to test on Linux and Windows too. I will get back to you once I have the test results.

@hoechenberger
Copy link
Contributor Author

@samruddhikhandale Tested and working on two separate macOS computers. I couldn't test on other operating systems, unfortunately!

Copy link
Member

@samruddhikhandale samruddhikhandale left a comment

Choose a reason for hiding this comment

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

Looks good to me~ Thank you!

@samruddhikhandale samruddhikhandale enabled auto-merge (squash) October 17, 2023 23:45
@samruddhikhandale samruddhikhandale merged commit 1155a99 into devcontainers:main Oct 17, 2023
51 of 53 checks passed
@hoechenberger hoechenberger deleted the ubuntu-keyserver branch October 18, 2023 05:36
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.

Python – Installation from source fails because GnuPG cannot import keys without user ID
2 participants