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

Remove unused http fallback code #379

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Anton-Kalpakchiev
Copy link
Collaborator

During Kraken's migration from HTTP to HTTPS, a mechanism was needed to fallback to HTTP if HTTPS failed for some reason. This migration has been finished for years, but the unused fallback code stayed. This PR cleans it up, resulting in less code to maintain and lower overall complexity.

Copy link
Collaborator

@gkeesh7 gkeesh7 left a comment

Choose a reason for hiding this comment

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

While Kraken's migration from HTTP to HTTPS did introduce a fallback mechanism to HTTP for failed HTTPS attempts, I don’t believe this code is unused. It actually aids in quickly unit testing the codebase without needing to configure TLS settings explicitly for HTTPS connections. Additionally, I suspect the failing builds on this PR might be due to the removal of a fix that was included in a previous PR.

@gkeesh7
Copy link
Collaborator

gkeesh7 commented Nov 14, 2024

--- FAIL: TestTagDownloadSuccess (0.00s)
    tagclient_test.go:66: 
                Error Trace:    tagclient_test.go:66
                Error:          Received unexpected error:
                                check blob exists: network error: Head "https://127.0.0.1:38263/v2/namespace-foo/repo-bar/manifests/nL8h8bE8": http: server gave HTTP response to HTTPS client
                Test:           TestTagDownloadSuccess
--- FAIL: TestTagDownloadFileNotFound (0.00s)
    tagclient_test.go:96: 
                Error Trace:    tagclient_test.go:96
                Error:          Not equal: 
                                expected: &errors.errorString{s:"blob not found"}
                                actual  : &errors.errorString{s:"check blob exists: network error: Head \"https://127.0.0.1:39683/v2/namespace-foo/repo-bar/manifests/wLjt8IvG\": http: server gave HTTP response to HTTPS client"}
                            
                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1,3 +1,3 @@
                                 (*errors.errorString)({
                                - s: (string) (len=14) "blob not found"
                                + s: (string) (len=158) "check blob exists: network error: Head \"https://127.0.0.1:39683/v2/namespace-foo/repo-bar/manifests/wLjt8IvG\": http: server gave HTTP response to HTTPS client"
                                 })
                Test:           TestTagDownloadFileNotFound

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.

2 participants