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

feat: Add Frequent Users feature in [AtlasProxy] #147

Merged
merged 4 commits into from
Jul 13, 2020
Merged

feat: Add Frequent Users feature in [AtlasProxy] #147

merged 4 commits into from
Jul 13, 2020

Conversation

mgorsk1
Copy link
Contributor

@mgorsk1 mgorsk1 commented Jul 6, 2020

Summary of Changes

This change enables Frequent Users feature in Table Detail View and My Profile section.

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-commenter
Copy link

codecov-commenter commented Jul 6, 2020 •
edited
Loading

Codecov Report

Merging #147 into master will increase coverage by 0.85%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #147      +/-   ##
==========================================
+ Coverage   72.06%   72.91%   +0.85%     
==========================================
  Files          26       26              
  Lines        1199     1233      +34     
  Branches      121      128       +7     
==========================================
+ Hits          864      899      +35     
+ Misses        308      307       -1     
  Partials       27       27
Impacted Files Coverage Δ
metadata_service/proxy/atlas_proxy.py 83.53% <100.00%> (+3.15%) ⬆️

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 19b5c7b...81aa8d1. Read the comment docs.

@feng-tao
Copy link
Member

feng-tao commented Jul 6, 2020

cc @verdan

@feng-tao
Copy link
Member

feng-tao commented Jul 8, 2020

hey. @verdan , could you help to take a look?

verdan
verdan requested changes Jul 8, 2020
View reviewed changes
Copy link
Member

@verdan verdan left a comment

Choose a reason for hiding this comment

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

@mgorsk1 a few comments!

metadata_service/proxy/atlas_proxy.py Outdated Show resolved Hide resolved
metadata_service/proxy/atlas_proxy.py Outdated Show resolved Hide resolved
metadata_service/proxy/atlas_proxy.py Outdated Show resolved Hide resolved
metadata_service/proxy/atlas_proxy.py Outdated Show resolved Hide resolved
metadata_service/proxy/atlas_proxy.py Show resolved Hide resolved
metadata_service/proxy/atlas_proxy.py Outdated Show resolved Hide resolved
metadata_service/proxy/atlas_proxy.py Outdated Show resolved Hide resolved
metadata_service/proxy/atlas_proxy.py Outdated Show resolved Hide resolved
@mgorsk1
Copy link
Contributor Author

mgorsk1 commented Jul 8, 2020

thanks for the review @verdan , addressed comments and provided fixes, good for the second look 💪

@mgorsk1
Copy link
Contributor Author

mgorsk1 commented Jul 8, 2020

also waiting for #146 to go in and will rebase, seems useful from our perspective

@mgorsk1 mgorsk1 force-pushed the feature/table_readers branch 3 times, most recently from 24a46c8 to b4db266 Compare July 9, 2020 06:38
@mgorsk1 mgorsk1 force-pushed the feature/table_readers branch from b4db266 to 4fc6f8c Compare July 9, 2020 06:39
verdan
verdan previously approved these changes Jul 10, 2020
View reviewed changes
Copy link
Member

@verdan verdan left a comment

Choose a reason for hiding this comment

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

some small nits! approved otherwise. 🙂

metadata_service/proxy/atlas_proxy.py Outdated Show resolved Hide resolved
metadata_service/proxy/atlas_proxy.py Outdated Show resolved Hide resolved
feng-tao
feng-tao previously approved these changes Jul 10, 2020
View reviewed changes
Copy link
Member

@feng-tao feng-tao left a comment

Choose a reason for hiding this comment

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

two nits, otherwise lgtm based on Verdan's review!

metadata_service/proxy/atlas_proxy.py Outdated Show resolved Hide resolved
metadata_service/proxy/atlas_proxy.py Show resolved Hide resolved
@mgorsk1 mgorsk1 force-pushed the feature/table_readers branch from 4597c67 to f94c5ac Compare July 11, 2020 12:03
@mgorsk1 mgorsk1 force-pushed the feature/table_readers branch from 52fa1ec to 81aa8d1 Compare July 11, 2020 12:13
@mgorsk1 mgorsk1 requested a review from feng-tao July 11, 2020 12:16
feng-tao
feng-tao approved these changes Jul 13, 2020
View reviewed changes
Copy link
Member

@feng-tao feng-tao left a comment

Choose a reason for hiding this comment

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

lgtm, thanks

@feng-tao feng-tao merged commit 5a8c117 into amundsen-io:master Jul 13, 2020
jerryzhu2007 pushed a commit to kylg/amundsenmetadatalibrary that referenced this pull request Aug 20, 2020
Merge commit '369685cc715e95af82dfa4dc14d0c58af8bb1ac9'
a53daca 
* commit '369685cc715e95af82dfa4dc14d0c58af8bb1ac9':
  chore: replace references to Lyft -> Amundsen (amundsen-io#174)
  feat: Data Owner Implementation of Atlas Proxy (amundsen-io#156)
  chore: fix docker push action (amundsen-io#172)
  chore: add docker publish action and remove travis (amundsen-io#171)
  chore: add pypi publish action (amundsen-io#170)
  fix: removing OidcConfig file and making statsd configurable through envrionment variable (amundsen-io#157)
  ci: add dependabot config (amundsen-io#169)
  Update repo name in travis file (amundsen-io#163)
  feat: Populate is_view property in AtlasProxy (amundsen-io#155)
  fix: Overlapping table name issue in Readers [AtlasProxy]
  feat: Add resource_reports field in Table API ( Atlas proxy) (amundsen-io#149)
  chore: apply license headers to all the source files (amundsen-io#153)
  feat: Add programmatic descriptions parser for [AtlasProxy] (amundsen-io#152)
  feat: Add Frequent Users feature in [AtlasProxy] (amundsen-io#147)
  feat: Implement configurable minimum number of readers for popular tables (amundsen-io#146)
  chore: update the email for the project (amundsen-io#148)

# Conflicts:
#	README.md
#	docs/configurations.md
#	docs/structure.md
#	metadata_service/config.py
#	metadata_service/oidc_config.py
#	metadata_service/proxy/neo4j_proxy.py
#	requirements.txt
#	setup.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@feng-tao feng-tao feng-tao approved these changes

@verdan verdan verdan left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mgorsk1 @codecov-commenter @feng-tao @verdan