Sidebranch: address transition issues on replication engine and actions #9010
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,26 @@ | ||
| <div class="replication-page" data-test-replication-page> | ||
| {{#if isLoadingData }} | ||
| <LayoutLoading /> | ||
| {{else}} | ||
| {{yield | ||
| (hash | ||
| header=(component 'replication-header' | ||
| data=model | ||
| title=replicationMode | ||
| isSecondary=isSecondary | ||
| secondaryId=replicationDetails.secondaryId | ||
| ) | ||
| dashboard=(component | ||
| 'replication-dashboard' | ||
| data=model | ||
| isSecondary=isSecondary | ||
| replicationDetails=replicationDetails | ||
| clusterMode=clusterMode | ||
| reindexingDetails=reindexingDetails | ||
| ) | ||
| isDisabled=isDisabled | ||
| message=message | ||
| ) | ||
| }} | ||
| {{/if}} | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,26 @@ | ||
| import { alias } from '@ember/object/computed'; | ||
| import { inject as service } from '@ember/service'; | ||
| import Controller from '@ember/controller'; | ||
| import { task, timeout } from 'ember-concurrency'; | ||
|
|
||
| export default Controller.extend({ | ||
| rm: service('replication-mode'), | ||
| replicationMode: alias('rm.mode'), | ||
| waitForNewClusterToInit: task(function*(replicationMode) { | ||
| // waiting for the newly enabled cluster to init | ||
| // this ensures we don't hit a capabilities-self error, called in the model of the mode/index route | ||
| yield timeout(1000); | ||
|
There was a problem hiding this comment. I don't love arbitrary timeouts, but I trust you that this was necessary. For diligence: is there a way to kick this off after the other thing has finished from the index route? Looks like it does the transition fine when the mode is There was a problem hiding this comment. It has everything to do with hitting the model on the route it's transitioning to. For DR it's not transitioning to the same route, only in this specific case (Enabling a Performance Secondary) does it follow this flow: There was a problem hiding this comment. i was thinking about this too -- an arbitrary timeout feels like it could have the potential to be flaky. i know you have tried many ideas, but have you considered moving the fetch for
i'm not super confident it this, but technically we are trying to compute the capabilities after the model hook returns the vault cluster with replication enabled, so it might fit. 🤷♀️ https://guides.emberjs.com/release/routing/controllers/#toc_where-and-when-to-use-controllers i'm not sure, but some guidance from others would be helpful here too -- @meirish, @DingoEatingFuzz, or @johncowen, do any of you have advice for us on how to ensure a network request in the model doesn't happen until the model is already "ready"? There was a problem hiding this comment. Not having looked massively at this code yet, my question would be: What's the 'signal' so you know when this is 'ready'? It sounds like there isn't one?
So a follow up question would be, whilst potentially horrible, is there something you could poll so you know when this is 'ready'? There was a problem hiding this comment. Hmm yeah that's tricky - I'd expect replication status or cluster status to tell you if it's bootstrapping (or some other status as it's changing). Alternatively does every request return a 40x error? If so you could poll until you don't get 40x's (not great, but more of signal like John mentioned). Or even poll the capabilities-self endpoint and catch the errors until it doesn't return one. |
||
| return this.transitionToRoute('mode', replicationMode); | ||
| }), | ||
| actions: { | ||
| onEnable(replicationMode, mode) { | ||
| if (replicationMode == 'dr' && mode === 'secondary') { | ||
| return this.transitionToRoute('vault.cluster'); | ||
| } | ||
| if (replicationMode === 'dr') { | ||
| return this.transitionToRoute('mode', replicationMode); | ||
| } | ||
| this.waitForNewClusterToInit.perform(replicationMode); | ||
Monkeychip marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }, | ||
| onDisable() { | ||
| return this.transitionToRoute('index'); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,23 @@ | ||
| <section class="section"> | ||
|
There was a problem hiding this comment. just added section and div to file so it wouldn't span the whole width while waiting to load. |
||
| <div class="container is-widescreen"> | ||
| {{#if (eq model.mode 'unsupported')}} | ||
| <PageHeader as |p|> | ||
| <p.levelLeft> | ||
| <h1 class="title is-3 has-text-grey"> | ||
| Replication unsupported | ||
| </h1> | ||
| </p.levelLeft> | ||
| </PageHeader> | ||
| <EmptyState | ||
| @title="The current cluster configuration does not support replication" | ||
| /> | ||
| {{else}} | ||
| <ReplicationSummary | ||
| @cluster={{model}} | ||
| @showModeSummary={{true}} | ||
| @onEnable={{action "onEnable"}} | ||
| @onDisable={{action "onDisable"}} | ||
| /> | ||
| {{/if}} | ||
| </div> | ||
| </section> | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This separates out a specific property for the mode displayed in the Header. We needed one that wasn't also moonlighting for a property from the URL (e.g. modeForUrl).
