Bug 2015833 - Fetch policies during engine initialization#459
Conversation
1rneh
force-pushed
the
bug-2015833-fetch-policies-on-engine-initialization
branch
from
b873762 to
01c3d4b
Compare
| } | ||
|
|
||
| const res = await lazy.ConsoleClient.getRemotePolicies(); | ||
| if (!res.policies) { |
There was a problem hiding this comment.
The previous can throw I think, so you want to catch too, not just nullcheck.
| ); | ||
| this._failed = true; | ||
| } else { | ||
| this._policies = res.policies; |
There was a problem hiding this comment.
Doesn't this need to do a call/trigger/notify to explicitly process the policies?
There was a problem hiding this comment.
_ingestPolicies() or triggerOnPoliciesChanges() ?
There was a problem hiding this comment.
Oh, yes. Good catch! I have to fetch the policies before calling
Let me verify, that that is correct and working.
jonathanmendez
left a comment
jonathanmendez
left a comment
There was a problem hiding this comment.
I agree with gcp's feedback but I'll let him handle the followup. Otherwise looks good.
|
|
||
| case "policies-startup": { | ||
| const initializedPromise = this._initialize(); | ||
| this.spinResolve(initializedPromise); |
There was a problem hiding this comment.
I'm assuming/guessing that the reason this uses a spinResolve instead of await is that observe is non-async and defined in niIObserver so you can't change that, combined with needing _initialize to finish before calling _runPoliciesCallbacks. Is it worth adding a small comment here explaining the same?
There was a problem hiding this comment.
Some background is probably in the original PR #321 (comment)
1rneh
deleted the
bug-2015833-fetch-policies-on-engine-initialization
branch
4 participants
No description provided.