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

support huge amount of datacubes #1004

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bikov
Copy link

@bikov bikov commented Dec 22, 2022

We have a large number of datasources in our use - and when trying to use Turnilo we are getting {"error": "Can't fetch settings", "message": "Invalid string length"} when querying /sources
This should help us to support our use case with a large number of datacubes by splitting datacubes API from clusters API and adding pagination to the /sources/dataCubes

@bikov bikov requested a review from a team as a code owner December 22, 2022 16:55
@mkuthan
Copy link
Member

mkuthan commented Dec 23, 2022

Thank you for the contribution, we'll review the changes. Due to a Christmas break you could expect the review in the beginning of the new year.

…usters and adding pagination to the `/sources/dataCubes`
@bikov bikov force-pushed the support-large-ammount-of-datasources branch from 13a888a to 4a0ade5 Compare December 26, 2022 14:17
let dataCubesResult: SerializedDataCube[] = [];
let isDone = false;
let batch = 0;
while (!isDone) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we could extract this to async iterator and with for await ... of. That's very loose idea though!

…usters and adding pagination to the `/sources/dataCubes` - CR fixes
@bikov
Copy link
Author

bikov commented Jan 4, 2023

@adrianmroz-allegro Thank you for the review - I've fixed the comments

@adrianmroz-allegro
Copy link
Contributor

@bikov oh, that's unfortunate :) We were discussing your PR on one of our meetings and I'm finishing my implementation of this feature. If that's ok with you I'd like to push my solution and get review from you.

@adrianmroz-allegro
Copy link
Contributor

@bikov We had long weekend there - sorry for delay. PR is here: #1012 I'm still working on good tests for this, because we won't be using this feature so we'd like to have some confidence there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants