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

Add namespace, before, and after to search endpoint #2556

Merged
merged 9 commits into from
Sep 14, 2023

Conversation

wslulciuc
Copy link
Member

Problem

The searchAPI does not support filtering search results by namespace or date. See related issue #2550

Solution

This PR updates the search endpoint to support the following query params:

  • namespace - will match jobs or datasets within the given namespace.
  • before - will match jobs or datasets before YYYY-MM-DD.
  • after - will match jobs or datasets after YYYY-MM-DD.

Related PRs: #2553, #2555

Checklist

  • You've signed-off your work
  • Your changes are accompanied by tests (if relevant)
  • Your change contains a small diff and is self-contained
  • You've updated any relevant documentation (if relevant)
  • You've included a one-line summary of your change for the CHANGELOG.md (Depending on the change, this may not be necessary).
  • You've versioned your .sql database schema migration according to Flyway's naming convention (if relevant)
  • You've included a header in any source code files (if relevant)

@boring-cyborg boring-cyborg bot added the api API layer changes label Jul 28, 2023
@boring-cyborg boring-cyborg bot added the spec label Jul 28, 2023
@wslulciuc wslulciuc changed the title Feature/filter search by namespace or date Add namespace, before, and after to search endpoint Jul 28, 2023
Add support for the following parameters in the SearchDao:
* `namespace` - match jobs or datasets within the given namespace.
* `before` - match jobs or datasets before YYYY-MM-DD.
* `after` - match jobs or datasets after YYYY-MM-DD.

Relates to issue: #2550

Signed-off-by: Tatiana Al-Chueyr <[email protected]>
Co-authored-by: Willy Lulciuc <[email protected]>
@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #2556 (5c698e2) into main (d0d35fa) will increase coverage by 0.01%.
The diff coverage is 90.00%.

@@             Coverage Diff              @@
##               main    #2556      +/-   ##
============================================
+ Coverage     83.31%   83.33%   +0.01%     
- Complexity     1289     1291       +2     
============================================
  Files           243      244       +1     
  Lines          5940     5940              
  Branches        280      279       -1     
============================================
+ Hits           4949     4950       +1     
  Misses          844      844              
+ Partials        147      146       -1     
Files Changed Coverage Δ
api/src/main/java/marquez/db/SearchDao.java 75.00% <75.00%> (ø)
api/src/main/java/marquez/api/SearchResource.java 100.00% <100.00%> (+11.11%) ⬆️
api/src/main/java/marquez/common/Utils.java 93.95% <100.00%> (+0.04%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines +61 to +62
@QueryParam("before") @Valid @Pattern(regexp = YYYY_MM_DD) @Nullable String before,
@QueryParam("after") @Valid @Pattern(regexp = YYYY_MM_DD) @Nullable String after) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wslulciuc Since App Wizard doesn't expose Instant out of the box, would it be a possibility to define a custom parameter Instant converter and register it up at the MarquezApp.java? Or would that be overkill for the problem we're solving here?

Copy link
Member Author

Choose a reason for hiding this comment

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

For now, we'll continue to use the approach taken in our runsAPI:

  Response markRunAs(@NonNull RunState runState, @QueryParam("at") String atAsIso) {
    runService.markRunAs(runId, runState, Utils.toInstant(atAsIso));
    return getRun();
  }

But, agree a more long term solution would be to define a custom query param.

@wslulciuc wslulciuc marked this pull request as ready for review September 14, 2023 17:54
Copy link
Member

@phixMe phixMe left a comment

Choose a reason for hiding this comment

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

Thanks

@wslulciuc wslulciuc merged commit 3bbcd80 into main Sep 14, 2023
3 checks passed
@wslulciuc wslulciuc deleted the feature/filter-search-by-namespace-or-date branch September 14, 2023 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api API layer changes spec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants