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

Delay starting the server API until the config has been loaded. #5104

Merged
merged 1 commit into from
Jul 6, 2023

Conversation

ericpromislow
Copy link
Contributor

Fixes #5099

Copy link
Contributor

@mook-as mook-as left a comment

Choose a reason for hiding this comment

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

Per Slack discussion, we're leaning towards changing things so the HTTP server only starts after the settings have completed loading (with all the checks) instead.

@ericpromislow ericpromislow changed the title Verify that settings have been loaded before servicing API requests involving them. Delay starting the server API until the config has been loaded. Jul 5, 2023
@ericpromislow
Copy link
Contributor Author

Running this command while starting up RD:

while : ; do
  echo list-settings
  ./resources/darwin/bin/rdctl list-settings
  echo $?
  sleep 0.1
  echo api settings
  ./resources/darwin/bin/rdctl api settings
  echo $?
  sleep 0.1
  echo put settings
  ./resources/darwin/bin/rdctl api /settings -X PUT -b '{"version": 9, "kubernetes": {"version": "moo"}}'
  echo $?
  sleep 0.1
  echo extension install
  ./resources/darwin/bin/rdctl extension install beeker
  echo $?
  sleep 0.1
done

I get this output, where the result rdctl sees goes immediately from connection refused to a working server.


list-settings
Error: Get "http://127.0.0.1:6107/v1/settings": dial tcp 127.0.0.1:6107: connect: connection refused
1
api settings
Error: Get "http://127.0.0.1:6107/v1/settings": dial tcp 127.0.0.1:6107: connect: connection refused
1
put settings
Error: Put "http://127.0.0.1:6107/v1/settings": dial tcp 127.0.0.1:6107: connect: connection refused
1
extension install
Error: Post "http://127.0.0.1:6107/v1/extensions/install?id=beeker": dial tcp 127.0.0.1:6107: connect: connection refused
1
list-settings
{
  "version": 9,
  "application": {
    "adminAccess": false,
    "debug": false,
    "extensions": {
…

Copy link
Contributor

@mook-as mook-as left a comment

Choose a reason for hiding this comment

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

The code looks fine, but there's a rebased copy of #5098 in the log. Please rebase on main and we can merge.

* 05eac06083e5382cc0301f1424e73f81b7826dc5 (pr/5104/head, origin/5099-flag-premature-requests) Verify that settings have been loaded before servicing API requests involving them.
* 4a1f4b104ba279c7bf2562988bd41e4f73dd77bb RDX: Expose API to main window
| * 120c0a1074063e427e47a440cdb7d332ef540efa (origin/main) Merge pull request #5098 from mook-as/extensions/expose-to-app
|/| 
| * df2a2f0ef11630755dc2d8f33bc7f5978b0b5064 (pr/5098/head, extensions/expose-to-app) RDX: Expose API to main window
* |   dc2a7b7a5a632c3386118c789af3798bf852c138 Merge pull request #5103 from jandubois/merge-1.9.1

…nvolving them.

Many of the http commands rely on having a loaded config, so we don't lose much by waiting.

Signed-off-by: Eric Promislow <epromislow@suse.com>
@ericpromislow
Copy link
Contributor Author

Done...

@mook-as mook-as merged commit dcb08a7 into main Jul 6, 2023
@mook-as mook-as deleted the 5099-flag-premature-requests branch July 6, 2023 22:01
jandubois added a commit to jandubois/rancher-desktop that referenced this pull request Jul 11, 2023
See rancher-sandbox#5104

So we don't need to check the output of `rdctl api /settings` for
it anymore.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
jandubois added a commit to jandubois/rancher-desktop that referenced this pull request Jul 12, 2023
See rancher-sandbox#5104

So we don't need to check the output of `rdctl api /settings` for
it anymore.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
jandubois added a commit to jandubois/rancher-desktop that referenced this pull request Jul 20, 2023
See rancher-sandbox#5104

So we don't need to check the output of `rdctl api /settings` for
it anymore.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
jandubois added a commit to jandubois/rancher-desktop that referenced this pull request Jul 20, 2023
See rancher-sandbox#5104

So we don't need to check the output of `rdctl api /settings` for
it anymore.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
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.

settings API may return "undefined"
2 participants