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.

Sign up for GitHub

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

Jump to bottom

#899: add limit of 1000 for size at bingo elastic #919

Merged
merged 12 commits into from
Feb 11, 2023
Merged

#899: add limit of 1000 for size at bingo elastic #919

merged 12 commits into from
Feb 11, 2023

Conversation

MysterionRise
Copy link
Collaborator

solving #899

@MysterionRise
Copy link
Collaborator Author

@mkviatkovskii could you review this?

AlexanderSavelyev
AlexanderSavelyev requested changes Feb 11, 2023
View reviewed changes
Copy link
Collaborator

@AlexanderSavelyev AlexanderSavelyev left a comment

Choose a reason for hiding this comment

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

generic question: assume this change will limit max number of molecules to be returned to 1000. But how does user will get all structures? Do we have pagination?

bingo/bingo-elastic/java/src/main/java/com/epam/indigo/elastic/ElasticStream.java Outdated Show resolved Hide resolved
bingo/bingo-elastic/java/src/test/java/com/epam/indigo/elastic/FullUsageMoleculeTest.java Outdated Show resolved Hide resolved
bingo/bingo-elastic/python/bingo_elastic/elastic.py Outdated Show resolved Hide resolved
@MysterionRise
Copy link
Collaborator Author

@AlexanderSavelyev this is purely about limit on page size. Paging implementation isn't completed yet. Issue - #905

@AlexanderSavelyev AlexanderSavelyev changed the title add limit of 1000 for size at bingo elastic #899: add limit of 1000 for size at bingo elastic Feb 11, 2023
AlexanderSavelyev
AlexanderSavelyev approved these changes Feb 11, 2023
View reviewed changes
Copy link
Collaborator

@AlexanderSavelyev AlexanderSavelyev left a comment

Choose a reason for hiding this comment

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

closes #899

@AlexanderSavelyev AlexanderSavelyev merged commit b170f79 into master Feb 11, 2023
@AlexanderSavelyev AlexanderSavelyev deleted the limit-1000 branch February 11, 2023 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexanderSavelyev AlexanderSavelyev AlexanderSavelyev approved these changes

@mkviatkovskii mkviatkovskii Awaiting requested review from mkviatkovskii

@khyurri khyurri Awaiting requested review from khyurri

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MysterionRise @AlexanderSavelyev