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

feat: Use query parameter for basic search [AtlasProxy] #105

Conversation

mgorsk1
Copy link
Contributor

@mgorsk1 mgorsk1 commented May 14, 2020

Summary of Changes

After successful merge of https://reviews.apache.org/r/72440/ to master it's now possible to use much more flexible query parameter to conduct search. Previously we couldn't use it because sorting results only worked with entityFilters clause that used different handler, was less performant, required specific indication of fields on which searching should be done and did no full text processing (like tokenization, analyzers). Query parameter enforces full text search on all immediate attributes.

Tests

What tests did you add or modify and why? If no tests were added or modified, explain why. Remove this line

Documentation

What documentation did you add or modify and why? Add any relevant links then remove this line

CheckList

Make sure you have checked all steps below to ensure a timely review.

  • PR title addresses the issue accurately and concisely. Example: "Updates the version of Flask to v1.0.2"
  • PR includes a summary of changes.
  • PR adds unit tests, updates existing unit tests, OR documents why no test additions or modifications are needed.
  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does
  • PR passes make test

@codecov-io
Copy link

codecov-io commented May 14, 2020

Codecov Report

Merging #105 into master will decrease coverage by 0.86%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #105      +/-   ##
==========================================
- Coverage   75.86%   75.00%   -0.87%     
==========================================
  Files          17       17              
  Lines         609      608       -1     
  Branches       75       75              
==========================================
- Hits          462      456       -6     
- Misses        121      127       +6     
+ Partials       26       25       -1     
Impacted Files Coverage Δ
search_service/proxy/atlas.py 49.03% <100.00%> (-5.25%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9a8cc9...7510640. Read the comment docs.

@mgorsk1
Copy link
Contributor Author

mgorsk1 commented May 14, 2020

cc @verdan @dwarszawski

@mgorsk1 mgorsk1 changed the title 🎉 Initial commit. Use query parameter for basic search [AtlasProxy] May 14, 2020
verdan
verdan previously approved these changes May 14, 2020
@stale
Copy link

stale bot commented May 29, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale stalebot believes this issue/PR is no longer active label May 29, 2020
@mgorsk1
Copy link
Contributor Author

mgorsk1 commented May 29, 2020

@feng-tao just noticed that should've been merged

@stale stale bot removed the stale stalebot believes this issue/PR is no longer active label May 29, 2020
feng-tao
feng-tao previously approved these changes May 29, 2020
@feng-tao
Copy link
Member

thanks @mgorsk1 for the ping, lgtm

@mgorsk1 mgorsk1 dismissed stale reviews from feng-tao and verdan via 3ae9525 May 29, 2020 16:08
@mgorsk1 mgorsk1 force-pushed the feature/atlas_search_using_query_not_filters branch from 7510640 to 3ae9525 Compare May 29, 2020 16:08
@mgorsk1 mgorsk1 changed the title Use query parameter for basic search [AtlasProxy] feat - Use query parameter for basic search [AtlasProxy] May 29, 2020
@feng-tao feng-tao changed the title feat - Use query parameter for basic search [AtlasProxy] feat: Use query parameter for basic search [AtlasProxy] May 29, 2020
@codecov-commenter
Copy link

Codecov Report

Merging #105 into master will decrease coverage by 0.85%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #105      +/-   ##
==========================================
- Coverage   76.01%   75.16%   -0.86%     
==========================================
  Files          17       17              
  Lines         613      612       -1     
  Branches       76       76              
==========================================
- Hits          466      460       -6     
- Misses        121      127       +6     
+ Partials       26       25       -1     
Impacted Files Coverage Δ
search_service/proxy/atlas.py 49.03% <100.00%> (-5.25%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c864e7...3ae9525. Read the comment docs.

@feng-tao feng-tao merged commit 9961fde into amundsen-io:master May 29, 2020
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.

5 participants