Docker registry default #657
Conversation
…tex /v4/info + workspaces
| try { | ||
| // TODO fetch this from new endpoint or maybe store this in the profile?? | ||
| const registryUrl = (new URL(profile.url)).hostname.replace('api', 'private-registry'); | ||
| // First try to see if we have of registries from /v4/info, grab the first one with isCortex == true |
There was a problem hiding this comment.
do we want any unit tests to check these scenarios?
There was a problem hiding this comment.
Yeah need to add some... I just added a quick fix cause it was blocking me from some other testing..
| // TODO fetch this from new endpoint or maybe store this in the profile?? | ||
| const registryUrl = (new URL(profile.url)).hostname.replace('api', 'private-registry'); | ||
| // First try to see if we have of registries from /v4/info, grab the first one with isCortex == true | ||
| let registryUrl = Object.values(profile?.registries ?? {}).find((v) => v?.isCortex === true)?.url; |
There was a problem hiding this comment.
If a cluster has 2 or more registries(one of them being private-registry.) then we are choosing the default only.. Maybe a warning to the user that a certain registry is being used of the 2 or more options.
The below warning comes up ONLY when profile.registries.url is undefined
There was a problem hiding this comment.
I don't know what a good alternative is though... unless we want to add a registry to cortex configure &&|| `cortex workspaces --registry so you can specify one. ??
Just using string replace isn't good enough though
|
@jgielstra-cs I've resolved merge conflicts so we can get this into develop soon |
| @@ -10,6 +10,9 @@ | |||
| "ecmaVersion": 2022 | |||
| }, | |||
| "extends": "airbnb-base", | |||
| "parserOptions": { | |||
There was a problem hiding this comment.
unsure if this is still needed. maybe not? @jgielstra-cs
Checklist:
Please check you fulfill ALL of the relevant checkboxes
npm testand it passes