TDL-15317: Fix Primary Key for Feature Events #48
Conversation
|
|
||
| def __init__(self, config): | ||
| super().__init__(config=config) | ||
| self.key_properties.append("day" if self.period == 'dayRange' else "hour") |
There was a problem hiding this comment.
@savan-chovatiya can you please write a unit test case for the same
There was a problem hiding this comment.
Added unit tests for the key_properties.
|
CircleCI build is failing currently as we have added a standard discover test to verify updated primary key but replication keys are not with expected inclusion currently. The replication key with valid inclusion is fixed as part of #45 . |
| self.assertEqual(feature_event_stream1.key_properties, expected_primary_keys) | ||
|
|
||
| # Reset key properties for other test as it's class variable | ||
| FeatureEvents.key_properties = ['feature_id', 'visitor_id', 'account_id', 'server', 'remote_ip', 'user_agent'] |
There was a problem hiding this comment.
@savan-chovatiya Test cases should not be dependent on each other.
Please include this statement at the starting of the next test case which requires this class variable in the reset mode
There was a problem hiding this comment.
I removed that comment and initialize key_properties with the default value at the start of the test.
| self.assertEqual(feature_event_stream2.key_properties, expected_primary_keys) | ||
|
|
||
| # Reset key properties for other test as it's class variable | ||
| FeatureEvents.key_properties = ['feature_id', 'visitor_id', 'account_id', 'server', 'remote_ip', 'user_agent'] |
There was a problem hiding this comment.
Same comment as above
There was a problem hiding this comment.
I removed that comment and initialize key_properties with the default value at the start of the test.
This PR is dependent on PR [#45](This PR is dependent on PR #45) |
|
|
@kspeer825 Updated the bookmark test to cover |
Description of change
TDL-15149: Use correct PK for feature_events stream
Manual QA steps
Risks
Rollback steps