Use default client when pinging registry. #188
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As this code doesn't check for status code, there's no need to use custom transports to authenticate. For what I see it's only checking there is no communication issue Signed-off-by: Marcos Lilljedahl <[email protected]>
marcosnils
changed the title
|
doesnt work because the insecure certs and such, needs to be the client that passes those! |
|
@jessfraz 👋. For what I see that function is not testing for any status code, I thought that you only wanted to the test that there's actually communication between the client and the registry. If no status code is checked, what's the point of sending certs if you're still going to get 200 or 401 depending if you're authorized?. Anyways, I just thought it was a nice and quick performance improvement hack given the fact that again the status code is not checked. 👍 |
|
ah yeah, I just noticed the tests fail, but happy to do it if everything
still works
…On Mon, Sep 16, 2019 at 3:20 PM Marcos Nils ***@***.***> wrote:
@jessfraz <https://github.com/jessfraz> 👋. For what I see that function
is not testing for any status code, I thought that you only wanted to the
test that there's actually communication between the client and the
registry. If no status code is checked, what's the point of sending certs
if you're still going to get 200 or 401 depending if youre authorized?.
Anyways, I just thought it was a nice and quick performance improvement
hack given the fact that again the status code is not checked. 👍
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#188?email_source=notifications&email_token=AALA23EE6TVXSS7OSKKDA5LQKABCHA5CNFSM4HTB5EB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD62WCPA#issuecomment-531980604>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AALA23GXV5DRHG4V5TP3PBLQKABCHANCNFSM4HTB5EBQ>
.
--
Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu <http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3>
|
|
Would like to give it a try. I couldn't get tests to run locally #187 😢 |
|
so sorry it might have been the stuff i updated on master, but let me know!
…On Mon, Sep 16, 2019 at 3:34 PM Marcos Nils ***@***.***> wrote:
Would like to give it a try. I couldn't get tests to run locally #187
<#187> 😢
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#188?email_source=notifications&email_token=AALA23BAD6K3VA3QFAHGRSDQKACY5A5CNFSM4HTB5EB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD62W7RI#issuecomment-531984325>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AALA23ES3CTMMD33CVBY5YLQKACY5ANCNFSM4HTB5EBQ>
.
--
Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu <http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As this code doesn't check for status code, there's
no need to use custom transports to authenticate.
For what I see it's only checking there is no communication issue
I couldn't run tests due to #187 but for what I see it shouldn't break anything.
Signed-off-by: Marcos Lilljedahl [email protected]