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

Remove openSearchBackendFlowEnabled toggle #545

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

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:

@idastambuk idastambuk requested a review from a team as a code owner January 16, 2025 11:08
@idastambuk idastambuk requested review from kevinwcyu and njvrzm and removed request for a team January 16, 2025 11:08
Copy link
Contributor

@iwysiu iwysiu left a comment

Choose a reason for hiding this comment

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

The removal looks good, but it seems like tests using the backend flow were also removed from opensearchDatasource.test.ts? (I marked a couple of them, but it's worth just going over the code)

src/opensearchDatasource.ts Outdated Show resolved Hide resolved
header = JSON.parse(parts[0]);
});

it('should translate index pattern to current day', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I notice we only kept the 'should not resolve the variable in the original alias field in the query', are the other tests just not applicable anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the others in the describe block test query building, which we do on the backend now, I don't think so

src/opensearchDatasource.test.ts Show resolved Hide resolved
src/opensearchDatasource.test.ts Show resolved Hide resolved
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.

2 participants