Skip to content

Conversation

@enochtangg
Copy link
Contributor

@enochtangg enochtangg commented Sep 14, 2022

Now that all queries sent to Snuba specify an Entity, we want to remove the legacy default_entity attribute in a Dataset.

This PR is responsible for the following:

  • Removed default_entity from base Dataset class and its get method.
  • Enforced jsonschema to require configs to specify entities all sub-property.
  • Added all_entities attribute to dataset base class and pass entity keys to child initialization
  • Removed all get_default_entity occurrences in tests and added support where needed
  • Creation of new testing eventstream and insert endpoints to remove default_entity and dataset parameter altogether. Plan to re-route to new endpoints are specified here: ref(MDC): Update Snuba eventstream and insert endpoints sentry#38877

@enochtangg enochtangg requested a review from a team as a code owner September 14, 2022 20:22
@enochtangg enochtangg force-pushed the remove_default_entity branch from 35df546 to 2687ad1 Compare September 14, 2022 20:29
@enochtangg enochtangg changed the title feat(mdc): Remove default entity from base Dataset class feat(mdc): Remove default entity from base Dataset class [WIP] Sep 14, 2022
@enochtangg enochtangg force-pushed the remove_default_entity branch from 2687ad1 to 02a0f28 Compare September 14, 2022 21:24
@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2022

Codecov Report

Base: 92.78% // Head: 92.77% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (f87b62a) compared to base (43853fd).
Patch coverage: 94.52% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3142      +/-   ##
==========================================
- Coverage   92.78%   92.77%   -0.01%     
==========================================
  Files         677      678       +1     
  Lines       30968    31007      +39     
==========================================
+ Hits        28733    28768      +35     
- Misses       2235     2239       +4     
Impacted Files Coverage Δ
snuba/datasets/configuration/dataset_builder.py 100.00% <ø> (ø)
snuba/datasets/configuration/json_schema.py 100.00% <ø> (ø)
...ests/datasets/configuration/test_dataset_loader.py 100.00% <ø> (ø)
snuba/web/views.py 71.62% <55.55%> (-0.38%) ⬇️
snuba/datasets/cdc/groupassignee.py 100.00% <100.00%> (ø)
snuba/datasets/cdc/groupedmessage.py 100.00% <100.00%> (ø)
snuba/datasets/dataset.py 100.00% <100.00%> (+3.44%) ⬆️
snuba/datasets/discover.py 98.18% <100.00%> (ø)
snuba/datasets/events.py 100.00% <100.00%> (ø)
snuba/datasets/functions.py 87.50% <100.00%> (ø)
... and 27 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@enochtangg enochtangg changed the title feat(mdc): Remove default entity from base Dataset class [WIP] feat(mdc): Remove default entity from base Dataset class Sep 15, 2022
@enochtangg enochtangg force-pushed the remove_default_entity branch 2 times, most recently from 9c20561 to 91c4d0f Compare September 15, 2022 15:35
@enochtangg enochtangg force-pushed the remove_default_entity branch 2 times, most recently from 2bd8a9c to e45d32c Compare September 15, 2022 16:48
@enochtangg enochtangg force-pushed the remove_default_entity branch from e45d32c to 14dcb99 Compare September 15, 2022 19:11
@enochtangg enochtangg merged commit 5d18075 into master Sep 19, 2022
@enochtangg enochtangg deleted the remove_default_entity branch September 19, 2022 14:51
enochtangg added a commit to getsentry/sentry that referenced this pull request Sep 19, 2022
This PR is responsible for updating the Snuba eventstream and insert
test endpoints
* From: `POST /tests/{dataset}/eventstream` To: `POST
/tests/{entity}/eventstream`
* From: `POST /tests/{dataset}/insert` To: `POST
/tests/entities/{entity}/insert`

This change is required due to the removal of the `default_entity`
attribute in the Dataset class in Snuba. As a result, every request sent
to Snuba (e.g. query, eventstream, etc.) must specify both a dataset and
entity.
The changes to the server endpoint is specified here:
getsentry/snuba#3142
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants