chore: Add repr function to BadgeMetadata #397
Conversation
Signed-off-by: dikshathakur3119 <dikshathakur@lyft.com>
dikshathakur3119
requested review from
allisonsuarez,
feng-tao and
jinhyukchang
as code owners
Signed-off-by: dikshathakur3119 <dikshathakur@lyft.com>
Signed-off-by: dikshathakur3119 <dikshathakur@lyft.com>
|
@dikshathakur3119 LGTM, but a different thought: do we need db_name and schema in badge model? It doesn't seem to get used anywhere? @allisonsuarez WDYT? |
Yeah I think you are right, I had added it when first writing the BadgeMetadata and wasn't sure if it would be necessary since the badge key doesn't contain any of this information, but since table, column, and dashboard make those connections already i didn't end up needing it in Badge I forgot to remove it 😅 |
Signed-off-by: dikshathakur3119 <dikshathakur@lyft.com>
databuilder/models/badge.py
Outdated
| @@ -65,6 +60,11 @@ def __init__(self, | |||
| self._node_iter = iter(self.create_nodes()) | |||
| self._relation_iter = iter(self.create_relation()) | |||
|
|
|||
| def __repr__(self) -> str: | |||
| return 'BadgeMetadata({!r}, {!r}, {!r})'.format(self.cluster, | |||
There was a problem hiding this comment.
is cluster being used anywhere?
There was a problem hiding this comment.
no it is not but I did not remove it thinking if we want different badge for different clusters. I can remove it though since w e are not storing this info anywhere
There was a problem hiding this comment.
I don't think we need it given the start_key uri should include cluster field (db://cluster.schema/table?) even if we have different clusters, the uri will be different?
Signed-off-by: dikshathakur3119 <dikshathakur@lyft.com>
Signed-off-by: dikshathakur3119 dikshathakur@lyft.com
Summary of Changes
Adding repr() function to BadgeMetadata so that can be used in unit tests to compare two instances of BadgeMetadata
Removed database and schema from BadgeMetadata since that information is not used anywhere
Tests
NA
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.
make test