ui: Refactor redux connect for enqueue ranges #45667
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
March 3, 2020 14:15
Previously, `handleEnqueueRange` was being added to the `mapDispatchToProps` portion of the call to `connect`. After some refactoring (most likely due to a redux upgrade, see 7f94c61) this was throwing an error as redux was expecting a plain object but was recieving a Promise. It's hard to get the whole story with both Thunk and Saga middleware, but it seems that the GRPC Promise doesn't fit a signature that any of the middleware can use and so it goes back to Redux, which rejects it since it is not an action. Given that none of this data needs to be in the store, and doesn't seem to try to dispatch any events, I am moving the entire functionality internally to the component and taking it out of Redux entirely. Fixes #44948 Release note (admin ui change): refactor redux data flow in enqueue range
See comments on issue #42888 It may not be possible to not have a timeout on this GPRC request, but in the meantime I'm increasing it to 1 hour to cover any reasonable case. Release note (admin ui change): increase enqueue range timeout to an hour to prevent operation from timing out before completion.
|
This change is |
`lodash.parseInt` aligns with the ES5 implementation of `parseInt` which is well suppported (see references). | references | | ---------- | | https://caniuse.com/#feat=mdn-javascript_builtins_parseint | | https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt#Browser_compatibility | Release note (admin ui change): swapping lodash parseInt for standard parseInt
koorosh
approved these changes
Mar 3, 2020
|
bors r+ |
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Moving
handleEnqueueRangefrom Redux connect to a method in the component as the data seems useless in the state and doesn't dispatch an action.It seems like the GPRC promise wasn't getting handled by either the Thunk or Saga middleware and was confusing Redux as with a Promise instead of an action.
I'm also increasing the request timeout time on the GPRC request to an hour to ensure completion of tasks.
fixes #44948, #42888