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: added TableStats model #1503

Merged
merged 7 commits into from
Sep 28, 2021
Merged

feat: added TableStats model #1503

merged 7 commits into from
Sep 28, 2021

Conversation

allisonsuarez
Copy link
Contributor

Signed-off-by: Allison Suarez Miranda asuarezmiranda@lyft.com

Summary of Changes

Added TableStats model which allows adding Stats directly to table nodes rather than only to table columns.

Tests

Documentation

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

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
@boring-cyborg boring-cyborg bot added area:databuilder From databuilder folder category:models labels Sep 24, 2021
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
@@ -94,7 +202,7 @@ def _create_node_iterator(self) -> Iterator[GraphNode]:
"""
node = GraphNode(
key=self.get_table_stat_model_key(),
Copy link
Member

Choose a reason for hiding this comment

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

Let's rename this function to get_column_stat_model_key?

RESOURCE_STAT_RELATION_TYPE = 'STAT'


class TableStats(GraphSerializable, TableSerializable):
Copy link
Member

Choose a reason for hiding this comment

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

What are the stats that we want to surface? There are some metadata on the left side panel in table details page, for example, total number of rows, etc. Will some of the metadata be moved to this stats model?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As of right now we are indexing usage stats like dau, query count, and service specific usage like for airflow, flyte, etc. In the future however we could use this to index stats about the data in the table as well, similar to how we already do for columns.

Copy link

@dkunitsk dkunitsk left a comment

Choose a reason for hiding this comment

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

lgtm

databuilder/databuilder/models/table_stats.py Show resolved Hide resolved
databuilder/databuilder/models/table_stats.py Outdated Show resolved Hide resolved
allisonsuarez and others added 4 commits September 27, 2021 11:56
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
@allisonsuarez allisonsuarez merged commit aeacfc8 into main Sep 28, 2021
@allisonsuarez allisonsuarez deleted the asm-stat-table-databuilder branch September 28, 2021 17:44
amommendes pushed a commit to amommendes/amundsen that referenced this pull request Jan 21, 2022
* feat: added TableStats model

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* lint

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* ...

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* implemented feedback

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* updated unit test

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
Signed-off-by: Amom Mendes <amommendes@hotmail.com>
ozandogrultan pushed a commit to deliveryhero/amundsen that referenced this pull request Apr 28, 2022
* feat: added TableStats model

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* lint

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* ...

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* implemented feedback

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* updated unit test

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
Signed-off-by: Ozan Dogrultan <ozan.dogrultan@deliveryhero.com>
zacr pushed a commit to SaltIO/amundsen that referenced this pull request May 13, 2022
* feat: added TableStats model

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* lint

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* ...

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* implemented feedback

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* updated unit test

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
hansadriaans pushed a commit to DataChefHQ/amundsen that referenced this pull request Jun 30, 2022
* feat: added TableStats model

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* lint

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* ...

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* implemented feedback

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* updated unit test

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:databuilder From databuilder folder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants