Open rustyjux opened 4 months ago
@Elson9 if you give https://github.com/bcgov/api-services-portal/pull/1123 a quick review you should be good to go to start this.
Heya had a little poke at your WIP. I don't think this is a gamebreaker right now but I noticed gateway current doesn't handle the situation well where the current gateway (set in the config) isn't available in the session (e.g. because the user switched from test to prod env and the gateways are different):
❯ just run gateway list
Display Name Gateway ID
basic2 gw-6eed7
qstart gw-3d6d2
RVINEGAR's Gateway gw-4701e
RVINEGAR's Gateway gw-87657
service val 2 gw-f2797
service-validation gw-6ce97
❯ just run config set gateway not-my-gateway
✓ Config settings saved
❯ just run gateway current
Next Steps:
Run gwa login to obtain another auth token
Error: Missing authorization scope. (403)
exit status 1
error: Recipe `run` failed on line 15 with exit code 1
In this case, the error is correct (Missing auth scope). However, I have a valid auth token, so the next step isn't accurate. Instead, I think I'd want to see the current gateway (from the config settings) somewhere in the error messaging, so that I realize it isn't in my list of gateways available from my session.
That current config gateway is still available from config get gateway, but because we have the shorter (and now improved w/ display name) option of gateway current, users might forget/not use it.
❯ just run config get gateway
not-my-gateway
The trick to implementing this would be to distinguish between response.StatusCode == http.StatusUnauthorized
because it's an old / invalid session vs. the gateway is not in the token scope.
On another note - did we discuss not supporting backwards API version compatibility in gwa-cli
?
I botched things up with stuff like: https://github.com/bcgov/gwa-cli/blob/d31387c50297846827491fc6ebc99dac703b5163/cmd/gateway.go#L202 where even though can pass v2, it wont work (as is) due to the changed route.
I see that you have taken a slightly different but also non-backwards compatible path with: https://github.com/bcgov/gwa-cli/blob/d31387c50297846827491fc6ebc99dac703b5163/cmd/gateway.go#L48 furthering a pattern introduced in https://github.com/bcgov/gwa-cli/blob/d31387c50297846827491fc6ebc99dac703b5163/cmd/generateConfig.go#L55 which is to specify the API version, rather than use %s.
Unless we feel otherwise, I'd say let's go with not ensuring backwards API compatibility, but continuing to use the version (%s
) let's us not change the version everywhere if/when we go to v4
Takeaways from chat:
Further, re comment above about not being logged in - https://github.com/bcgov/gwa-cli/issues/111#issuecomment-2291567666
When token is actually expired:
❯ just run gateway current
Error: invalid_grant
Token is not active
exit status 1
It doesn't show the 'Next steps' message.
Changing the API version isn't currently implemented in the NewConfigSetCmd (only gateway, token, host, and scheme). We could remove the instructions on setting the global API version (which doesn't work) and then v3 of the CLI will always use v3 of the API.
@ikethecoder what do you think about pegging v3 cli to v3 API? (and keeping this going forward to v4 etc) It's probably the simplest approach and the cases where users would want to mix n match are rare and not really necessary?
re: pegging v3 cli to v3 API : Agreed
@Elson9 I think adding some loading indicators for the sometimes-slow checking the service step would be nice to include.
Service validation loading: https://github.com/bcgov/gwa-cli/commit/d692373de3f6b10b04ed5a78cf2a01fd17fb3e76
still waiting on review on https://github.com/bcgov/gwa-cli/pull/122
Show display names in
gwa gateway current
andgwa gateway list
.For
current
, will need to store gateway displayName locally in .gwa-configBlocked by https://github.com/bcgov/api-services-portal/issues/1080