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

Bump Arroyo to 2.17.6 #6289

Merged
merged 6 commits into from
Sep 17, 2024
Merged

Bump Arroyo to 2.17.6 #6289

merged 6 commits into from
Sep 17, 2024

Conversation

ayirr7
Copy link
Member

@ayirr7 ayirr7 commented Sep 11, 2024

Bumped the Python requirements, not sure if anything needs to be changed on the Rust side because of https://github.com/getsentry/snuba/blob/master/rust_snuba/Cargo.toml#L34

@ayirr7 ayirr7 requested a review from a team as a code owner September 11, 2024 17:51
@untitaker
Copy link
Member

you also need to run cargo update -p. also, you need to wait for getsentry/pypi to finish before CI can pass

@ayirr7 ayirr7 changed the title Bump Arroyo to 2.17.5 Bump Arroyo to 2.17.6 Sep 11, 2024
@untitaker untitaker self-assigned this Sep 12, 2024
Copy link

codecov bot commented Sep 12, 2024

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
2456 1 2455 4
View the top 1 failed tests by shortest run time
tests.subscriptions.test_scheduler_consumer test_scheduler_consumer
Stack Traces | 0.015s run time
Traceback (most recent call last):
  File ".../tests/subscriptions/test_scheduler_consumer.py", line 97, in test_scheduler_consumer
    scheduler = builder.build_consumer()
                ^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../snuba/subscriptions/scheduler_consumer.py", line 278, in build_consumer
    self.__build_tick_consumer(),
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../snuba/subscriptions/scheduler_consumer.py", line 310, in __build_tick_consumer
    return CommitLogTickConsumer(
           ^^^^^^^^^^^^^^^^^^^^^^
TypeError: Can't instantiate abstract class CommitLogTickConsumer with abstract method member_id

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

Copy link
Member

@onkar onkar left a comment

Choose a reason for hiding this comment

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

Did we do any testing to make sure that the new version works as expected? Please share the testing details in the PR.

@untitaker
Copy link
Member

@onkar the changes in arroyo are not significant enough to cause concern like this. CI should be enough.

Copy link
Member

@untitaker untitaker left a comment

Choose a reason for hiding this comment

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

thank you for your contribution!

@untitaker untitaker merged commit 40bf550 into master Sep 17, 2024
30 checks passed
@untitaker untitaker deleted the bump-arroyo branch September 17, 2024 16:32
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.

3 participants