Fixes the issue of url creation for same component when no name is provided #1061
Conversation
|
Code Climate has analyzed commit 7c94a4c and detected 0 issues on this pull request. View more on Code Climate. |
mik-dass
force-pushed
the
url_multiple_ports
branch
2 times, most recently
from
009e356
to
d65c429
Compare
Codecov Report
@@ Coverage Diff @@
## master #1061 +/- ##
==========================================
- Coverage 41.53% 41.46% -0.07%
==========================================
Files 30 30
Lines 4081 4085 +4
==========================================
- Hits 1695 1694 -1
- Misses 2212 2218 +6
+ Partials 174 173 -1
Continue to review full report at Codecov.
|
|
✅ odo build 634 completed and artifacts can be found here (for commit df39f66ad2 by @mik-dass) |
mik-dass
force-pushed
the
url_multiple_ports
branch
from
d65c429
to
4b9c80b
Compare
|
✅ odo build 649 completed and artifacts can be found here (for commit cc8ff74c12 by @mik-dass) |
mik-dass
force-pushed
the
url_multiple_ports
branch
from
4b9c80b
to
c9feace
Compare
|
✅ odo build 650 completed and artifacts can be found here (for commit 2b72fd66f1 by @mik-dass) |
mik-dass
force-pushed
the
url_multiple_ports
branch
from
c9feace
to
bb617fc
Compare
|
✅ odo build 651 completed and artifacts can be found here (for commit 3edf64b459 by @mik-dass) |
mik-dass
force-pushed
the
url_multiple_ports
branch
from
bb617fc
to
38d78cb
Compare
|
✅ odo build 652 completed and artifacts can be found here (for commit 3f9e69b32f by @mik-dass) |
mik-dass
force-pushed
the
url_multiple_ports
branch
from
38d78cb
to
de794a9
Compare
|
✅ odo build 653 completed and artifacts can be found here (for commit 0b967f2dd4 by @mik-dass) |
mik-dass
force-pushed
the
url_multiple_ports
branch
from
de794a9
to
af33aa5
Compare
|
✅ odo build 654 completed and artifacts can be found here (for commit 434058fa5a by @mik-dass) |
pkg/odo/cli/url/url.go
Outdated
| @@ -63,10 +63,15 @@ The created URL can be used to access the specified component from outside the O | |||
| applicationName := context.Application | |||
| componentName := context.Component() | |||
|
|
|||
| componentPort, err := url.GetValidPortNumber(client, urlPort, componentName, applicationName) | |||
| if err != nil { | |||
There was a problem hiding this comment.
No need to check here, this is done in CheckError.
pkg/url/url.go
Outdated
|
|
||
| if portNumber == -1 { | ||
| if len(componentPorts) > 1 { | ||
| return portNumber, errors.Errorf("'port' is required as the component %s exposes %d ports: %s", componentName, len(componentPorts), strings.Trim(strings.Replace(fmt.Sprint(componentPorts), " ", ",", -1), "[]")) |
There was a problem hiding this comment.
This is bad as the error message leaks the UI port flag to this layer which shouldn't know about it. The error message should be more generic and processed at the command level to inform the user that the port flag is required.
There was a problem hiding this comment.
I didn't understand it
There was a problem hiding this comment.
The error message mentions port which is the flag that the command uses when this function is called to pass the port number. This means that the error message in this function is tied to the calling command implementation. This should be fixed.
There was a problem hiding this comment.
More precisely, the error message should mention the missing function parameter portNumber and not something called port, which doesn't mean anything in this context.
pkg/url/url.go
Outdated
| var portFound bool | ||
|
|
||
| if portNumber == -1 { | ||
| if len(componentPorts) > 1 { |
pkg/url/url.go
Outdated
| } | ||
| } | ||
|
|
||
| if !portFound { |
There was a problem hiding this comment.
No need for this: if port was found then the function should have already returned.
mik-dass
force-pushed
the
url_multiple_ports
branch
from
af33aa5
to
bf0fe09
Compare
|
✅ odo build 691 completed and artifacts can be found here (for commit 088251732c by @mik-dass) |
|
@metacosm Fixed Please have a look again :) |
…ovided It introduces a new function GetValidPortNumber to check and return the valid port of the component. Signed-off-by: mik-dass <mrinald7@gmail.com>
Signed-off-by: mik-dass <mrinald7@gmail.com>
Signed-off-by: mik-dass <mrinald7@gmail.com>
mik-dass
force-pushed
the
url_multiple_ports
branch
from
f43e8dc
to
7c94a4c
Compare
|
✅ odo build 693 completed and artifacts can be found here (for commit 09cab3f1b9 by @mik-dass) |
fixes #1040
It introduces a new function GetValidPortNumber() to check and return the valid port of the component.