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

Support alternative AMQP ports #1331

Merged
merged 2 commits into from
Nov 1, 2023
Merged

Support alternative AMQP ports #1331

merged 2 commits into from
Nov 1, 2023

Conversation

chris-janidlo
Copy link
Contributor

Description

[sc-27144]

Fixes # (issue)

Type of change

  • New feature (non-breaking change that adds functionality)

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #27144: Support configuring an endpoint to use AMQPS over 443.

@chris-janidlo chris-janidlo force-pushed the 443-ampqs branch 2 times, most recently from de536f0 to 2ad2038 Compare October 25, 2023 19:15
@chris-janidlo
Copy link
Contributor Author

@khk-globus brought up some good use-cases for ports that are outside of what the web service supports, so I dropped the code that restricts ports in favor of just documenting which ones we support. (Still need to verify that the ones mentioned in the docs here are accurate.)

Copy link
Contributor

@khk-globus khk-globus left a comment

Choose a reason for hiding this comment

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

I'm wondering if it would be easier to modify just the initial connection string? That is, when we receive the URL, do the urlparse()/urlunparse() dance. That would avoid the need to change any of the code in rabbit_mq/, interchange.py, or executor.py.

Copy link
Contributor

@khk-globus khk-globus left a comment

Choose a reason for hiding this comment

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

Woo woo!

@chris-janidlo chris-janidlo merged commit 77f2160 into main Nov 1, 2023
30 checks passed
@chris-janidlo chris-janidlo deleted the 443-ampqs branch November 1, 2023 19:07
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