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

V2 route handling #1149

Merged
merged 9 commits into from
Jan 2, 2025
Merged

V2 route handling #1149

merged 9 commits into from
Jan 2, 2025

Conversation

mieciu
Copy link
Member

@mieciu mieciu commented Jan 2, 2025

While working on #1119 I noticed that a lot of this code could already safely land in main branch and act as "general bridge" between the present and v2 api.

Notable changes are switching to interfaces (LogManagerIface/QueryRunnerIFace). Therefore we could introduce the intermediate layer in route handlers for queries which can use both old and new structures (new log manager using v2 api, etc.).

Also, the _field_caps got more specific as it no longer accept full Quesma config to be more suited for being called within a processor (which just has index config).

@mieciu mieciu marked this pull request as ready for review January 2, 2025 10:56
@mieciu mieciu requested a review from a team as a code owner January 2, 2025 10:56
Copy link
Contributor

@pdelewski pdelewski left a comment

Choose a reason for hiding this comment

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

Please switch to v2 temporarily, by changing quesma_v2 = true in main.go. It seems that unit tests require update.

go test
# quesma/quesma [quesma/quesma.test]
./search_common_table_test.go:332:25: queryRunner.handleSearch undefined (type *QueryRunner has no field or method handleSearch, but does have method HandleSearch)
./search_norace_test.go:56:16: queryRunner.handleSearch undefined (type *QueryRunner has no field or method handleSearch, but does have method HandleSearch)
./search_norace_test.go:128:22: queryRunner.handleSearch undefined (type *QueryRunner has no field or method handleSearch, but does have method HandleSearch)
./search_opensearch_test.go:64:27: queryRunner.handleSearch undefined (type *QueryRunner has no field or method handleSearch, but does have method HandleSearch)
./search_opensearch_test.go:200:31: queryRunner.handleSearch undefined (type *QueryRunner has no field or method handleSearch, but does have method HandleSearch)
./search_test.go:95:26: queryRunner.handleAsyncSearch undefined (type *QueryRunner has no field or method handleAsyncSearch, but does have method HandleAsyncSearch)
./search_test.go:146:26: queryRunner.handleAsyncSearch undefined (type *QueryRunner has no field or method handleAsyncSearch, but does have method HandleAsyncSearch)
./search_test.go:337:26: queryRunner.handleSearch undefined (type *QueryRunner has no field or method handleSearch, but does have method HandleSearch)
./search_test.go:397:26: queryRunner.handleSearch undefined (type *QueryRunner has no field or method handleSearch, but does have method HandleSearch)
./search_test.go:441:23: queryRunner.handleSearch undefined (type *QueryRunner has no field or method handleSearch, but does have method HandleSearch)
./search_test.go:441:23: too many errors

Copy link
Contributor

@pdelewski pdelewski left a comment

Choose a reason for hiding this comment

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

LGTM :)

@mieciu mieciu added this pull request to the merge queue Jan 2, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 2, 2025
@mieciu
Copy link
Member Author

mieciu commented Jan 2, 2025

@mieciu mieciu added this pull request to the merge queue Jan 2, 2025
Merged via the queue into main with commit f30d0c9 Jan 2, 2025
5 checks passed
@mieciu mieciu deleted the v2-route-handling branch January 2, 2025 12:12
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