BUG: fix error when establishing plugin connections with protocols in all caps. #1236
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nstelter-slac
force-pushed
the
protocol_fix
branch
2 times, most recently
from
23accd0 to
6461bb7
Compare
nstelter-slac
requested review from
YektaY,
jbellister-slac and
craftablescience
nstelter-slac
force-pushed
the
protocol_fix
branch
from
6461bb7 to
2e1c604
Compare
… all caps
We only defined and only accept the protocol in channel addresses when its all lowercase. ('pva', 'calc', 'ca', etc)
Used in a while address, it looks like: 'ca://test:example'.
But sometimes we set the protocol to be uppercase, like in the
testing when we use 'CA://MTEST'.
This fix just converts all protocols into lowercase to avoid an error,
which imo is more user friendly than enforcing case-sensitive protocols
Found this issue when running tests on pyside6 since with it logger.error() calls are printed and fail
the tests, but on pyqt5 we need to set 'log_cli=true' to get the same
behavior. (will set this in followup patch)
nstelter-slac
force-pushed
the
protocol_fix
branch
from
2e1c604 to
90c4807
Compare
craftablescience
approved these changes
Apr 24, 2025
jbellister-slac
approved these changes
Apr 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We only defined and only accept the protocol in channel addresses when its all lowercase. ('pva', 'calc', 'ca', etc)
Used in an address, it looks like: 'ca://test:example'.
But sometimes we set the protocol to be uppercase, like in the testing when we use 'CA://MTEST'.
This fix just converts all protocols into lowercase to avoid an error, which imo is more user friendly than enforcing case-sensitive protocols
Found this issue when running tests on pyside6 since with it logger.error() calls are printed and fail the tests, but on pyqt5 we need to set 'log_cli=true' to get the same behavior. (which we currently don't do, but i'll set it in followup patch)