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

Preview DLQ Events (GSI-1127) #144

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

TheByronHimes
Copy link
Member

@TheByronHimes TheByronHimes commented Nov 26, 2024

ExtractedEventInfo

  • The type_ assignment used to fall back to the header value, but didn't remove it. The result is that the "type" would be found both in the top-level type_ variable as well as in headers["type"]. Now, the value is popped.

KafkaDLQSubscriber

  • Removed the subclassing of InboundProviderBase because this is actually a specialized class that has little overlap with the normal subscriber provider class.
  • ._get_events_from_dlq():
    • New method to get the events from the DLQ without hanging. Called by .preview(), .ignore(), and .process().
    • max_records (int) parameter tells the function how many records to retrieve at most.
    • Uses AIOKafkaConsumer.getmany with max_records and an arbitrary but reasonable timeout (500ms). The purpose of the timeout is to retrieve the records without hanging forever, since we can't know if or how many records will be available to fetch.
    • This method does not commit offsets itself, because that behavior is dependent on the use case.
  • .preview():
    • Returns the next events in the DLQ topic without affecting offsets
    • The value used for max_records is limit + skip
    • Has 2 parameters: limit (int) and skip (int). These parameters are for pagination of the preview results.
      limit will cap the number of results returned, and skip tells the function how far forward to start the preview. If there are no events in the topic, or if skip is greater than the number of events available, an empty list is returned.
  • .ignore():
    • Replaces run(ignore=True)
    • If no events are found in the DLQ, nothing happens.
    • If an event is found, the offsets are committed and nothing more happens. This effectively "ignores" the event.
  • process():
    • Replaces run(ignore=False)
    • If no events are found in the DLQ, nothing happens.

Tests

  • TEST_DLQ_EVENT was created because several tests were defining the same test event to publish directly to the DLQ. Adding this allowed for the condensing of the relevant test cases.
  • 3 test functions were added to test the preview functionality
  • 1 test function was added to verify the timeout behavior of the methods when there is nothing in the DLQ topic.

@TheByronHimes TheByronHimes requested a review from Cito November 26, 2024 09:59
@TheByronHimes TheByronHimes marked this pull request as ready for review November 26, 2024 09:59
@TheByronHimes TheByronHimes changed the title Feature/preview dlq events gsi 1127 Preview DLQ Events (GSI-1127) Nov 26, 2024
@coveralls
Copy link

coveralls commented Nov 26, 2024

Pull Request Test Coverage Report for Build 12052771316

Details

  • 32 of 32 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 92.421%

Totals Coverage Status
Change from base Build 11973418599: 0.1%
Covered Lines: 2073
Relevant Lines: 2243

💛 - Coveralls

@TheByronHimes TheByronHimes marked this pull request as draft December 16, 2024 13:12
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.

2 participants