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

Revert API request default timeout seconds #8812

Conversation

KeisukeYamashita
Copy link
Member

@KeisukeYamashita KeisukeYamashita commented Dec 21, 2020

What

Increase the timeout seconds from

1.24.0 default timeout

(SETTINGS.pollSchedule || 3000) * 2 + 5000,

to

< 1.24.0 default timeout

(SETTINGS.pollSchedule || 30000) * 2 + 5000,

Why

Solves spinnaker/spinnaker#6246.

This change was released in 1.24.0 in this PR (Deprecate API.one('foo').all('bar').get() in favor of REST('/foo/bar').get() ). I think this changed happened unintentionally during this refactor and I want to revert it because GetManifest request often requires duration of seconds.

When I upgraded to Spinnaker 1.24.0, the server group detail page often hung with loading because the browser GetManifest request is timeouts and retries again and again.

Screen Shot 2020-12-21 at 23 40 03

@spinnakerbot
Copy link
Contributor

The following commits need their title changed:

  • f317a29: Increase the timeout for api request

Please format your commit title into the form:

<type>(<scope>): <subject>, e.g. fix(kubernetes): address NPE in status check

This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here.

Signed-off-by: KeisukeYamashita <19yamashita15@gmail.com>
@KeisukeYamashita KeisukeYamashita force-pushed the KeisukeYamashita/fix-api-request-timeout branch from f317a29 to 42a9f24 Compare December 21, 2020 16:35
@christopherthielen christopherthielen added the ready to merge Reviewed and ready for merge label Dec 21, 2020
@mergify mergify bot merged commit c32c91a into spinnaker:master Dec 21, 2020
@KeisukeYamashita
Copy link
Member Author

KeisukeYamashita commented Dec 21, 2020

Thank you for your quick review.

BTW, I fixed the typo in my description above:

I think this changed happened intentionally during this refactor ...

to

I think this changed happened unintentionally during this refactor ...

Sorry for my typo.

@KeisukeYamashita KeisukeYamashita deleted the KeisukeYamashita/fix-api-request-timeout branch December 21, 2020 17:10
vigneshm added a commit that referenced this pull request Jan 13, 2021
95eacfb refactor(core/deployment): Update redblack fields without force updating (#8840)
a027d62 fix(gitlab): fix help text for gitlab artifacts
8858746 feat(md): waiting status (#8836)
c7eb9f4 feat(kubernetes): Raw resources UI MVP (#8800)
ef87a9e fix(core/executions): Update migrated status to match API (#8831)
3a51af2 fix(core/deploymentStrategy): do not show highlander preview in deploy stage config (only show in clone dialog)
8be06a0 feat(core/deploymentStrategy): Add a preview for Highlander deploys
c32c91a fix(serverGroup): Increase the timeout for api request (#8812)
29a85a0 feat(core/executions): Render newly migrated execution groups  (#8807)
8f291c0 fix(core/projects): Fix duplicate Projects appearing in recent history (on search screen) (#8806)
mergify bot pushed a commit that referenced this pull request Jan 13, 2021
95eacfb refactor(core/deployment): Update redblack fields without force updating (#8840)
a027d62 fix(gitlab): fix help text for gitlab artifacts
8858746 feat(md): waiting status (#8836)
c7eb9f4 feat(kubernetes): Raw resources UI MVP (#8800)
ef87a9e fix(core/executions): Update migrated status to match API (#8831)
3a51af2 fix(core/deploymentStrategy): do not show highlander preview in deploy stage config (only show in clone dialog)
8be06a0 feat(core/deploymentStrategy): Add a preview for Highlander deploys
c32c91a fix(serverGroup): Increase the timeout for api request (#8812)
29a85a0 feat(core/executions): Render newly migrated execution groups  (#8807)
8f291c0 fix(core/projects): Fix duplicate Projects appearing in recent history (on search screen) (#8806)
paragbhingre pushed a commit to paragbhingre/deck that referenced this pull request Jan 21, 2021
95eacfb refactor(core/deployment): Update redblack fields without force updating (spinnaker#8840)
a027d62 fix(gitlab): fix help text for gitlab artifacts
8858746 feat(md): waiting status (spinnaker#8836)
c7eb9f4 feat(kubernetes): Raw resources UI MVP (spinnaker#8800)
ef87a9e fix(core/executions): Update migrated status to match API (spinnaker#8831)
3a51af2 fix(core/deploymentStrategy): do not show highlander preview in deploy stage config (only show in clone dialog)
8be06a0 feat(core/deploymentStrategy): Add a preview for Highlander deploys
c32c91a fix(serverGroup): Increase the timeout for api request (spinnaker#8812)
29a85a0 feat(core/executions): Render newly migrated execution groups  (spinnaker#8807)
8f291c0 fix(core/projects): Fix duplicate Projects appearing in recent history (on search screen) (spinnaker#8806)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Reviewed and ready for merge target-release/1.25
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants