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: Add Column Badge API #273

Merged
merged 36 commits into from
Mar 19, 2021

Conversation

alldoami
Copy link
Contributor

@alldoami alldoami commented Mar 6, 2021

Summary of Changes

Added Column Badge API that addresses this issue: amundsen-io/amundsen#944

Tests

Tested exactly how we're testing Table Badges right now.

Documentation

amundsen-io/amundsen#962

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

@alldoami alldoami changed the title [feature] Add Column Badge API [feat] Add Column Badge API Mar 6, 2021
@alldoami alldoami changed the title [feat] Add Column Badge API feat: Add Column Badge API Mar 6, 2021
@alldoami alldoami force-pushed the adoami/badge-column-api branch from 2baa8a8 to c69b0d0 Compare March 6, 2021 01:39
@codecov-io
Copy link

codecov-io commented Mar 6, 2021

Codecov Report

Merging #273 (fd812db) into master (2752492) will increase coverage by 3.54%.
The diff coverage is 73.34%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #273      +/-   ##
==========================================
+ Coverage   74.10%   77.64%   +3.54%     
==========================================
  Files          25       27       +2     
  Lines        1255     1387     +132     
  Branches      136      163      +27     
==========================================
+ Hits          930     1077     +147     
+ Misses        297      263      -34     
- Partials       28       47      +19     
Impacted Files Coverage Δ
metadata_service/api/popular_tables.py 100.00% <ø> (ø)
metadata_service/api/system.py 66.66% <ø> (ø)
metadata_service/api/user.py 100.00% <ø> (ø)
metadata_service/proxy/statsd_utilities.py 81.25% <ø> (ø)
metadata_service/util.py 100.00% <ø> (ø)
metadata_service/proxy/shared.py 28.57% <28.57%> (ø)
metadata_service/api/badge.py 61.29% <61.29%> (ø)
metadata_service/proxy/neo4j_proxy.py 71.55% <61.53%> (-3.45%) ⬇️
metadata_service/api/column.py 72.22% <64.28%> (-27.78%) ⬇️
metadata_service/proxy/base_proxy.py 67.07% <73.33%> (-0.07%) ⬇️
... and 17 more

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 681893f...fd812db. Read the comment docs.

@alldoami alldoami force-pushed the adoami/badge-column-api branch from 6e6f1aa to 752888b Compare March 6, 2021 02:12
@feng-tao feng-tao requested a review from allisonsuarez March 6, 2021 06:21
@alldoami alldoami force-pushed the adoami/badge-column-api branch 2 times, most recently from 346cd0c to 56eb593 Compare March 8, 2021 21:42
@alldoami alldoami marked this pull request as ready for review March 9, 2021 02:08
@alldoami
Copy link
Contributor Author

alldoami commented Mar 9, 2021

Would love to get Dmitriy's code to get merged fixing amundsen-io/amundsen#939 before merging this.

@dkunitsk
Copy link

dkunitsk commented Mar 9, 2021

amundsen-io/amundsen#939 should be fixed now (by amundsen-io/amundsen#951)

@alldoami
Copy link
Contributor Author

alldoami commented Mar 9, 2021

Thanks @dkunitsk! Would love to get this in the new release then 😄

Copy link
Contributor

@dorianj dorianj left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

metadata_service/proxy/neo4j_proxy.py Outdated Show resolved Hide resolved
metadata_service/proxy/neo4j_proxy.py Outdated Show resolved Hide resolved
@feng-tao feng-tao added the keep fresh Disables stalebot from closing an issue label Mar 13, 2021
Signed-off-by: adoami <[email protected]>
Signed-off-by: adoami <[email protected]>
Signed-off-by: adoami <[email protected]>
Signed-off-by: adoami <[email protected]>
Signed-off-by: adoami <[email protected]>
Signed-off-by: adoami <[email protected]>
…#279)

* fixes pip install and configs

Signed-off-by: verdan <[email protected]>

* cleanup

Signed-off-by: verdan <[email protected]>

* test fix

Signed-off-by: verdan <[email protected]>

* removes the requirements changes from branch

Signed-off-by: verdan <[email protected]>
Signed-off-by: adoami <[email protected]>
@alldoami alldoami force-pushed the adoami/badge-column-api branch 5 times, most recently from 1af0217 to 054e1c4 Compare March 16, 2021 21:49
Signed-off-by: adoami <[email protected]>
Signed-off-by: adoami <[email protected]>
Signed-off-by: adoami <[email protected]>
Signed-off-by: adoami <[email protected]>
Signed-off-by: adoami <[email protected]>
Signed-off-by: adoami <[email protected]>
Signed-off-by: adoami <[email protected]>
@alldoami alldoami force-pushed the adoami/badge-column-api branch from 054e1c4 to e8e27b2 Compare March 16, 2021 21:59
Copy link
Contributor

@dorianj dorianj left a comment

Choose a reason for hiding this comment

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

Nice!

metadata_service/api/swagger_doc/column/badge_put.yml Outdated Show resolved Hide resolved
Signed-off-by: adoami <[email protected]>
@alldoami alldoami force-pushed the adoami/badge-column-api branch from a41ceff to 3cd9a4d Compare March 18, 2021 07:36
@alldoami alldoami requested a review from feng-tao March 18, 2021 07:37
@feng-tao
Copy link
Member

thanks @alldoami

@feng-tao feng-tao merged commit ee0ac63 into amundsen-io:master Mar 19, 2021
@alldoami
Copy link
Contributor Author

Thanks @feng-tao! Should we merge amundsen-io/amundsen#962 too? I fixed the merge history 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep fresh Disables stalebot from closing an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants