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

New search endpoint #6910

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

New search endpoint #6910

wants to merge 1 commit into from
+1,745 −206

Conversation

aeSouid
Copy link
Contributor

@aeSouid aeSouid commented Jan 23, 2025

WIP:
Update all the search endpoints to use the new implementation.
Create a new binding annotation to parse filters in the new format.
Update the JDBC implementation to handle operation-specific filters.

TODO:
Fix the unit tests.
Finalize changes for specific fields in the search functionality.

@loicmathieu
Copy link
Member

Thinking about it lately, and we sometimes have a /search endpoint and sometimes a /find. I think instead of updating the existing endpoints (which would be a breaking change) like it is done, it's better to create new endpoints /filter and deprecate the old /search or /find when it's no longer used by the UI.

By this, all endpoints named would be aligned.

@fhussonnois
Copy link
Member

I'm sharing the point of view of @loicmathieu , we should not introduce a breaking change on the /search API.

  • Some customers could be impacted
  • This would need to have a bigbang update of the UI

Existing endpoints should just map the params to the new filters and deprecate existing ones. Agree, that we should align the naming for endpoints but not sure we have to deprecate existing /search. I think all current params like query, scope are optional so:

  • if the new filters param is non-empty use it.
  • Otherwise, if existing params are not null, map them to the new QueryFilter object.

Then, in a few versions all deprecated params would have been removed.

I think that's what Ludo had in mind.

@aeSouid aeSouid force-pushed the feat/query-filter-operation branch from 13562f1 to 4ec16c5 Compare January 31, 2025 14:22
@aeSouid
Copy link
Contributor Author

aeSouid commented Jan 31, 2025

@loicmathieu @fhussonnois For the /search endpoints, I kept the old parameters and added the new QueryFilters as suggested. So now, you can use both the old and the new parameter format.

@anna-geller anna-geller added the kind/do-not-merge Don't merge label Feb 3, 2025
@anna-geller
Copy link
Member

this shouldn't be merged in 0.21 as it's not fully finished topic - FE related issue #7051

loicmathieu
loicmathieu requested changes Feb 3, 2025
View reviewed changes
jdbc/src/main/java/io/kestra/jdbc/repository/AbstractJdbcTriggerRepository.java Outdated Show resolved Hide resolved
webserver/src/main/java/io/kestra/webserver/controllers/api/ExecutionController.java Show resolved Hide resolved
webserver/src/main/java/io/kestra/webserver/utils/RequestUtils.java Show resolved Hide resolved
webserver/src/main/java/io/kestra/webserver/utils/RequestUtils.java Show resolved Hide resolved
webserver/src/main/java/io/kestra/webserver/utils/RequestUtils.java Show resolved Hide resolved
webserver/src/main/java/io/kestra/webserver/utils/TimeLineSearchUtils.java Outdated Show resolved Hide resolved
webserver/src/main/java/io/kestra/webserver/utils/TimeLineSearchUtils.java Outdated Show resolved Hide resolved
@loicmathieu
Copy link
Member

Two important things to enderstand:

  • startDate and endDate both should apply to the same field, whether the field is startDate/EndDate/creationDat/updatedDate. They express the period to filter on a single field.
  • timeRange also apply to a single field but is better to be an API only concept that maps to the standard startDate/endDate filters to avoid duplicating them.

@aeSouid aeSouid force-pushed the feat/query-filter-operation branch from 1397e32 to 7b083c8 Compare February 3, 2025 12:11
@anna-geller anna-geller removed the kind/do-not-merge Don't merge label Feb 3, 2025
@aeSouid aeSouid force-pushed the feat/query-filter-operation branch from 06f991f to 49ca572 Compare February 4, 2025 10:25
@aeSouid
Feat Search:
f90aa7d 
Create operation enum for search
Add custom parser for filter[field][operation]=value format
create new Annotation @QueryFilterFormat to bind to specfic pojo

Feat filter:
Implement new abstract rapo method
Implement the find elasticsearch implementation

Feat filter WIP:
Implement the find search method of the flow repository

Feat filter WIP:
Create new enum for all fields
Add the list of fields with their allowed operation

Feat filter WIP:
expose list fields with their supported operation in the config api

Feat filter WIP:
Handle scope and lables in the query filters mapper

Feat Filter (WIP):
change the binder parser to handle key value format
change all /search endpoint
change all filter jdbc implementation based on the filters list

Feat Filter (WIP):
Add the @inherited annotation to propagate to the override methods

Feat Filter (WIP):
Fix the abstract unit tests

Feat Filter (WIP):
Handle the state.type fields

Feat Filter (WIP):
Handle the childfilter fields
handle to snake case column name

Feat Filter (UNIT TEST REFACTORING):
handle to snake case column names

Feat Filter (UNIT TEST REFACTORING):
refacto columnName with DSL.quoteName

Feat Search (Fix Unit Test) :
refactor the geenric method filter repo
use the specific labels condition

Feat Search (Fix Unit Test) :

use the specific labels condition for execution lables

Feat Search (Fix Unit Test) :

use the specific labels condition for execution labels

Feat Search (Fix Unit Test) :

use the specific labels condition for execution labels

Refactoring:
code clean up

Unit test Fix:
Fix the specific log level filter

Unit test Fix:
Refactor Execution controller to handle time range and dates

Unit test Fix:
Refactor task controller to handle time range and dates

Unit test Fix:
Refactor flow controller test for the search endpoint

Unit test Fix:
Refactor log controller unit test for the new /search endpoint

Refactor:
Add method extract query from list filters

Change the scope list to mutable

Feat Filter:
Add scope handling in jdbc query

Feat Filter:
fix cast to ChildFilter

Feat Filter:
Synchronise with develop

Feat Filter:
Rollback deprecated params for all /search endpoints

Feat Filters:
Fix the received list of values in filter params

Feat Filters:
Add unit test for utils classes

Feat Filters:
Remove unused query params

Refacto Filter:
Create new QueryFilter utils class
add deprecated on all fields /Search endpoint

Refactor unit test AbstractExecutionRepository

Use only one filter for end_date and start_date

Use only one filter for end_date and start_date
@aeSouid aeSouid force-pushed the feat/query-filter-operation branch from 49ca572 to f90aa7d Compare February 4, 2025 10:29
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Feb 4, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
70.4% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@aeSouid aeSouid requested a review from loicmathieu February 4, 2025 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fhussonnois fhussonnois Awaiting requested review from fhussonnois

@loicmathieu loicmathieu Awaiting requested review from loicmathieu

Assignees
No one assigned
Labels
None yet
Projects
Pull Requests
Status: To review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aeSouid @loicmathieu @fhussonnois @anna-geller