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

Multiple queriesv3 #152

Closed
wants to merge 2 commits into from
Closed

Multiple queriesv3 #152

wants to merge 2 commits into from

Conversation

idastambuk
Copy link
Contributor

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

@github-actions
Copy link

Backend code coverage report for PR #152
No changes

@github-actions
Copy link

Frontend code coverage report for PR #152

Plugin Main PR Difference
src 77.77% 77.79% .02%

Copy link
Member

@sarahzinger sarahzinger left a comment

Choose a reason for hiding this comment

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

seems neat code wise, but taking a step back, what is the problem we're trying to solve here? Like what is the intended user experience we are fixing?

I just made multiple queries for example and got back an error:
Screenshot 2023-04-24 at 11 27 25 AM

but honestly I'm not terribly confident I understand what the experience should be for multiple queries you know?

@idastambuk
Copy link
Contributor Author

Closing this after thinking about this line:
const er = new OpenSearchResponse([target], res);
and realizing it would probably be not so simple to extract responses that are in response.aggregations and response.hits that correspond to the respective queries. That is, without refactoring OpenSearchResponse.ts and probably making it more unreadable.
I think this PR is a less impactful solution for now

@idastambuk idastambuk closed this May 3, 2023
@idastambuk idastambuk deleted the multiple-queriesv3 branch May 3, 2023 07:31
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.

None yet

2 participants