Integrates Atlas DSL Search #17

whazor · 2019-05-02T14:35:12Z

Title

My PR Title addresses the issue accurately and concisely.
- Example: "Updates the version of Flask to v1.0.2"
- In case you are adding a dependency, check if the license complies with the ASF 3rd Party License Policy.

Description

Here are some details about my PR:
Similarly as in the metadata service we want to provide an integration with Apache Atlas, but then for search.

In order to have a second integration inside of the search service, we add a base proxy in order to have more than one possible search engine.

Furthermore, Apache Atlas provides multiple methods of searching and we discuss them within docs/atlas-search.md. We choose to search with the DSL as it is straightforward offered from Atlas and still provides some flexibility in search.

Tests

My PR adds the following unit tests:
tests/unit/proxy/atlas.py

Which tests both normal search and with fields. The unit test mocks the atlas client and adds support for checking the query send to Atlas.

Commits

I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters (not including Jira issue reference)
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"

Documentation

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

Code Quality & Coverage

Passes make test

feng-tao · 2019-05-02T17:27:04Z

exciting :) let us know once it is ready to review :)
cc @verdan @jinhyukchang

feng-tao · 2019-05-02T17:27:36Z

oh CLA check seems to fail. I think you may want to configure your github username locally and do one more push.

whazor · 2019-05-03T07:07:49Z

Rebased changes with the latest config change. Cc: @gschmutz environment variables will change. Does anyone want me to add backwards compatibility?

verdan

Awesome work @whazor 👍 👍
A few comments/questions for my understanding.

search_service/proxy/atlas.py

codecov-io · 2019-05-09T09:07:49Z

Codecov Report

Merging #17 into master will increase coverage by 9.32%.
The diff coverage is 89.92%.

@@            Coverage Diff             @@
##           master      #17      +/-   ##
==========================================
+ Coverage   67.12%   76.45%   +9.32%     
==========================================
  Files           8       10       +2     
  Lines         216      344     +128     
  Branches       19       37      +18     
==========================================
+ Hits          145      263     +118     
- Misses         62       67       +5     
- Partials        9       14       +5

Impacted Files	Coverage Δ
search_service/proxy/elasticsearch.py	`66.29% <100%> (+0.38%)`	⬆️
search_service/config.py	`100% <100%> (ø)`	⬆️
search_service/api/search.py	`35.13% <20%> (ø)`	⬆️
search_service/proxy/base.py	`77.77% <77.77%> (ø)`
search_service/proxy/atlas.py	`92.66% <92.66%> (ø)`

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 86ea652...12ea887. Read the comment docs.

whazor · 2019-05-09T09:10:24Z

I updated the branch in the same way as I did in metadatalibrary. Currently this pull request works together with that pull request.

feng-tao · 2019-05-10T00:35:59Z

cc @verdan @jinhyukchang

whazor · 2019-05-10T09:12:23Z

@verdan @feng-tao @jinhyukchang updated tests and config, now supports both master from metadatalibrary and the pull request. By default the configuration is set to ATLAS_NAME_ATTRIBUTE = 'qualifiedName' which will make it work for the master. Should be ready to be merged.

Note that ATLAS_NAME_ATTRIBUTE on name in general does not work for master of metadatalibrary without my pull request.

verdan

Just a few minor comments, but overall really nice work Nanne.

verdan · 2019-05-10T10:11:49Z

search_service/proxy/atlas.py

+
+                db_name = db_attrs.get(self.NAME_ATTRIBUTE)
+                db_qualified_name = db_attrs.get('qualifiedName', '').split('@')
+                db_cluster = db_qualified_name[1] if len(db_qualified_name) > 1 else ''


Database entity has an attribute clusterName, which can be used directly here.

verdan · 2019-05-10T10:13:19Z

search_service/proxy/atlas.py

+                          key=f"{entity.typeName or 'Table'}://{db_cluster}.{db_name}/{table_name}",
+                          description=attrs.get('description'),
+                          cluster=db_cluster,
+                          database=entity.typeName or 'Table',


There would always be a typeName. So this check would never go to or

search_service/proxy/atlas.py

docs/atlas-search.md

search_service/config.py

feng-tao · 2019-05-13T21:42:36Z

@whazor we should bump the pypi version as well. Let me know once the pr is updated, and we could merge it. Thanks.

* Add new proxy with the Apache Atlas as backend * Add document to describing Apache Atlas search methods

whazor · 2019-05-14T06:08:13Z

@feng-tao Resolved all issues, thank you for bumping the pypi version. Sorry all for the inconvenience of renaming the elastic env variables.

feng-tao · 2019-05-14T15:52:57Z

no worry, thanks @whazor . exciting to see Amundsen works with Atlas end-to-end :)

feng-tao · 2019-05-14T15:59:29Z

Merge :) Thanks @whazor
cc @verdan @bolkedebruin

…_metrics_dashboards to test * commit '6ffc57032ef83197b3d642ccad02b31a633c0d18': Add metrics/dashboards Fix #24, correct initialisation of elastic search (amundsen-io#27) [DPTOOLS-2252] Publish Docker image in CI (amundsen-io#26) Integrates Atlas DSL Search (amundsen-io#17) Update PULL_REQUEST_TEMPLATE.md (amundsen-io#23) Update README.md (amundsen-io#22) Add codecov based for search repo (amundsen-io#20) Update README.md (amundsen-io#19) Set the elasticsearch base (endpoint) from env variable (amundsen-io#16) Adds the PR template for amundsen search service (amundsen-io#15) Doc fix: Docker pull the official image (amundsen-io#14) Changed the name of this file for consistency (amundsen-io#13) gitignore dist/ as in metadataservice PR #28 (amundsen-io#12)

whazor force-pushed the atlas branch 3 times, most recently from d81bc7d to 8895f1a Compare May 3, 2019 07:04

verdan requested changes May 3, 2019

View reviewed changes

whazor force-pushed the atlas branch 3 times, most recently from 30200b8 to 68fcd84 Compare May 9, 2019 09:06

Add base proxy behind elastic search to support other search engines

749ade6

whazor force-pushed the atlas branch 2 times, most recently from 130a932 to ccbd764 Compare May 10, 2019 09:08

verdan approved these changes May 10, 2019

View reviewed changes

feng-tao approved these changes May 11, 2019

View reviewed changes

docs/atlas-search.md Outdated Show resolved Hide resolved

search_service/config.py Outdated Show resolved Hide resolved

Add support for Apache Atlas DSL search

fb0ff54

* Add new proxy with the Apache Atlas as backend * Add document to describing Apache Atlas search methods

whazor force-pushed the atlas branch from ccbd764 to fb0ff54 Compare May 14, 2019 06:04

Update setup.py

12ea887

feng-tao merged commit 1a05eb2 into amundsen-io:master May 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrates Atlas DSL Search #17

Integrates Atlas DSL Search #17

whazor commented May 2, 2019 •

edited

Loading

feng-tao commented May 2, 2019

feng-tao commented May 2, 2019

whazor commented May 3, 2019

verdan left a comment •

edited

Loading

codecov-io commented May 9, 2019 •

edited

Loading

whazor commented May 9, 2019

feng-tao commented May 10, 2019

whazor commented May 10, 2019

verdan left a comment

verdan May 10, 2019

verdan May 10, 2019

feng-tao commented May 13, 2019

whazor commented May 14, 2019

feng-tao commented May 14, 2019

feng-tao commented May 14, 2019

Integrates Atlas DSL Search #17

Integrates Atlas DSL Search #17

Conversation

whazor commented May 2, 2019 • edited Loading

Title

Description

Tests

Commits

Documentation

Code Quality & Coverage

feng-tao commented May 2, 2019

feng-tao commented May 2, 2019

whazor commented May 3, 2019

verdan left a comment • edited Loading

Choose a reason for hiding this comment

codecov-io commented May 9, 2019 • edited Loading

Codecov Report

whazor commented May 9, 2019

feng-tao commented May 10, 2019

whazor commented May 10, 2019

verdan left a comment

Choose a reason for hiding this comment

verdan May 10, 2019

Choose a reason for hiding this comment

verdan May 10, 2019

Choose a reason for hiding this comment

feng-tao commented May 13, 2019

whazor commented May 14, 2019

feng-tao commented May 14, 2019

feng-tao commented May 14, 2019

whazor commented May 2, 2019 •

edited

Loading

verdan left a comment •

edited

Loading

codecov-io commented May 9, 2019 •

edited

Loading