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

Fixed bug in QtRemoteDispatcher #192

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

cjtitus
Copy link
Contributor

@cjtitus cjtitus commented Jan 30, 2024

Description

The QtRemoteDispatcher class had an outdated name for one of the objects inside the dispatcher it was creating, which caused the entire class to crash when used. Fixing the name (_bluesky_consumer.consumer -> _bluesky_consumer._consumer) restores functionality.

See the code in https://github.com/bluesky/bluesky-kafka/blob/main/bluesky_kafka/__init__.py#L149 and bluesky/bluesky-kafka@e9001b7 which changed from consumer to _consumer

How Has This Been Tested?

Tested with a containerized beamline running Kafka. Confirmed that QtRemoteDispatcher now instantiates correctly, and dispatches correctly.

…tdated and causing QtRemoteDispatcher to crash
@tacaswell
Copy link
Contributor

95% the linting is because we did not pin the version of black and the new version changes the formatting a bit.

Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

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

Looks good to me.
@cjtitus, do you mind applying the black formatting with the new version? Then we'll merge.

@cjtitus
Copy link
Contributor Author

cjtitus commented Feb 1, 2024

What version of black do you want me to use? The actual latest one that's causing the lint check to fail? Cause then this PR goes from touching one name in one file, to touching a quarter of the code-base...

@mrakitin mrakitin merged commit 362778f into bluesky:master Feb 2, 2024
1 of 3 checks passed
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