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

ref(replays): remove unused chunking code #47877

Merged
merged 4 commits into from
May 2, 2023

Conversation

JoshFerge
Copy link
Member

After merging getsentry/relay#2032, we've observed that we are no longer processing chunked messages (as intended). We can remove the logic for this as it is no longer used.

@JoshFerge JoshFerge requested a review from a team as a code owner April 24, 2023 23:00
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 24, 2023
@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

Merging #47877 (dd5f7fd) into master (159428a) will increase coverage by 2.52%.
The diff coverage is 100.00%.

❗ Current head dd5f7fd differs from pull request most recent head 7558772. Consider uploading reports for the commit 7558772 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #47877      +/-   ##
==========================================
+ Coverage   78.29%   80.81%   +2.52%     
==========================================
  Files        4762     4762              
  Lines      201354   201285      -69     
  Branches    11594    11594              
==========================================
+ Hits       157644   162667    +5023     
+ Misses      43454    38362    -5092     
  Partials      256      256              
Impacted Files Coverage Δ
src/sentry/replays/consumers/recording.py 94.73% <100.00%> (-1.01%) ⬇️
src/sentry/replays/usecases/ingest/__init__.py 80.00% <100.00%> (-3.57%) ⬇️

... and 316 files with indirect coverage changes

with metrics.timer("replays.process_recording.store_recording.drop_segments"):
parts.drop()


@metrics.wraps("replays.usecases.ingest.ingest_recording_not_chunked")
def ingest_recording_not_chunked(
Copy link
Member

Choose a reason for hiding this comment

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

This should be renamed to ingest_recording. The other ingest_recording function can be renamed to _ingest_recording or merged into the body of this function.

def move_chunks_to_cache_or_skip(message: Message[KafkaPayload]) -> MessageContext:
"""Move chunk messages to cache or skip."""
def initialize_message_context(message: Message[KafkaPayload]) -> MessageContext:
"""initialize a Sentry transaction and unpack the message."""
Copy link
Member

Choose a reason for hiding this comment

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

Nit: capitalization.

@JoshFerge JoshFerge enabled auto-merge (squash) May 2, 2023 16:37
@JoshFerge JoshFerge merged commit fa6fdea into master May 2, 2023
@JoshFerge JoshFerge deleted the jferg/remove-chunking-behavior branch May 2, 2023 17:15
@github-actions github-actions bot locked and limited conversation to collaborators May 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants