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

feat: request data in descending order #30

Merged
merged 1 commit into from
Jan 7, 2022
Merged

Conversation

NorbertNader
Copy link
Contributor

@NorbertNader NorbertNader commented Jan 7, 2022

Overview

  • Request data in descending order
  • Fix testing ground regression after rebase

Before
before_320

After
after_320

Tests

Updated unit tests

Legal

This project is available under the Apache 2.0 License.

@NorbertNader NorbertNader added the enhancement New feature or request label Jan 7, 2022
@NorbertNader NorbertNader self-assigned this Jan 7, 2022
@@ -62,10 +62,13 @@ export const dataReducer: Reducer<DataStreamsStore, AsyncActions> = (
// TODO: clean this to one single source of truth cache
const requestCache = streamStore != null ? streamStore.requestCache : EMPTY_CACHE;

// We always want data in ascending order in the cache
const sortedData = getDataPoints(data, data.resolution).sort((a, b) => a.x - b.x);
Copy link
Contributor

Choose a reason for hiding this comment

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

we always want it in order in the cache, but I think we should also always keep them in order in the DataStream. Could we instead solve this more towards the service layer, so that the reducer instead can remain unchanged and not need to sort? It should instead be the job of the 'SiteWise data source' to call the success handler with the data in the correct order if possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I was actually considering that as well. I didn't go down that route to create a "pit of success" and instead make sure that data will always be in ascending order, now matter what order it comes in from the service layer. What I am worried about is how we can enforce the data to be sorted. Why do you think it is better to sort it at the service layer?

@NorbertNader NorbertNader merged commit ffbbe19 into main Jan 7, 2022
@NorbertNader NorbertNader deleted the req-data-in-desc-order branch January 7, 2022 20:22
TheEvilDev pushed a commit that referenced this pull request Nov 2, 2022
Co-authored-by: Norbert Nader <nnader@amazon.com>
This was referenced Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants