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

bug-1900646: fix submitter to only consume standard queue #6635

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

willkg
Copy link
Contributor

@willkg willkg commented Jun 4, 2024

This fixes the submitter to only consume from the standard subscription rather than from standard, priority, and reprocessing.

It does it by stomping on the subscription_name settings for priority and reprocessing in the PubSubCrashQueue class. That means this will never run in AWS, but that's what our plan was anyhow.

@willkg willkg requested a review from a team as a code owner June 4, 2024 17:43
@willkg willkg requested a review from relud June 4, 2024 17:43
This fixes the submitter to only consume from the standard subscription
rather than from standard, priority, and reprocessing.

It does it by stomping on the subscription_name settings for priority
and reprocessing in the PubSubCrashQueue class. That means this will
never run in AWS, but that's what our plan was anyhow.
@willkg willkg force-pushed the willkg-bug-1900646-submitter-fix branch from 3c1201d to 522d9d7 Compare June 4, 2024 17:44
@@ -244,7 +261,11 @@ def publish(self, queue, crash_ids):
try:
future.result()
except Exception:
logger.exception(f"Crashid failed to publish: {batch[i]}")
logger.exception(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an unrelated fix to a bad exception logging line I did earlier. Boo me.

Copy link
Member

@relud relud left a comment

Choose a reason for hiding this comment

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

lgtm, but after reading i realize we could forgo this complexity and instead specify the same subscription for every queue in helm. i'm happy either way.

@willkg
Copy link
Contributor Author

willkg commented Jun 4, 2024

This PR is larger than it otherwise would have been because I had to change a bunch of things to make the submitter tests only run in gcp mode. Without those changes, it's like 50 LOC or something like that.

The complexity is limited to three spots and it's easily tested. I think this is ok to keep around for now.

@willkg willkg merged commit 2b8d996 into main Jun 4, 2024
2 checks passed
@willkg willkg deleted the willkg-bug-1900646-submitter-fix branch June 5, 2024 15:05
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