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

Add an option to auto-use default HTTP(S) port for Lavalink #5629

Merged
merged 6 commits into from Jun 19, 2023

Conversation

Drapersniper
Copy link
Contributor

@Drapersniper Drapersniper commented Mar 21, 2022

Description of the changes

Users are now allowed to set port to -1 which will automatically set the connection ports to 80/443 depending on the security settings.

Sorry but you cannot test the 443 part of it as it is added on #5593

Have the changes in this PR been tested?

Yes

@Drapersniper
Copy link
Contributor Author

What to test

Scenario 1:
[p]llset host lavalink.oops.wtf
[p]llset port -1
[p]llset secured # Needs to be set to secured

It should connect successfully

Scenario 2:
[p]llset host lavalink-coders.ml
[p]llset port -1
[p]llset secured # Needs to be set to unsecured

Scenario 3
Repeat Scenario 1 or 2 but have the incorrect [p]llset secured value
It should fail to connect

Scenario 4
Straight after scenario 3 change the secure state [p]llset secure to the correct value.

Scenario 5
[p]llset port docstring should tell users to set the value to -1 to set it to "None"

What to expect

1 - Scenario 1 user is able to connect successfully and play music
2 - Scenario 2 user is able to connect successfully and play music
3 - Scenario 3 connection will fail, it will attempt up to 10 times and then give up, Audio should say "Connection to Lavalink lost"
4 - Audio should tell the user to run [p]audioset restart, after running it user should be able to connect and play music
5 - Docstring is clear and directs the user on how to set the port to None

@Jackenmen Jackenmen self-assigned this Mar 29, 2022
@Jackenmen Jackenmen added this to the 3.5.0 milestone Mar 29, 2022
@Jackenmen Jackenmen changed the title Update audio to use https://github.com/Cog-Creators/Red-Lavalink/pull/121 Add a way for Audio to auto-use default HTTP(S) port to connect to Lavalink Mar 29, 2022
@Jackenmen Jackenmen removed their assignment Mar 31, 2022
@Jackenmen Jackenmen added the Type: Enhancement Something meant to enhance existing Red features. label Mar 31, 2022
@Drapersniper Drapersniper force-pushed the rll_121 branch 2 times, most recently from 8e5509b to 89a86d6 Compare March 31, 2022 10:42
@Drapersniper Drapersniper force-pushed the rll_121 branch 2 times, most recently from e0f5998 to b4c489c Compare April 3, 2022 10:23
@github-actions github-actions bot added the Category: Docs - Other This is related to documentation that doesn't have its dedicated label. label Nov 25, 2022
Copy link
Member

@Jackenmen Jackenmen left a comment

Choose a reason for hiding this comment

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

I think it would have been clearer if, instead of "None", the "Port" field said something like "Default HTTP(S) port".
image

@Jackenmen Jackenmen self-assigned this Dec 29, 2022
@Flame442 Flame442 modified the milestones: 3.5.0, 3.5.1 Apr 13, 2023
@github-actions github-actions bot removed the Category: Docs - Other This is related to documentation that doesn't have its dedicated label. label Jun 19, 2023
Copy link
Member

@Jackenmen Jackenmen left a comment

Choose a reason for hiding this comment

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

Thanks!

@Jackenmen Jackenmen changed the title Add a way for Audio to auto-use default HTTP(S) port to connect to Lavalink Add an option to auto-use default HTTP(S) port for Lavalink Jun 19, 2023
@Jackenmen Jackenmen merged commit 9d04f17 into Cog-Creators:V3/develop Jun 19, 2023
16 checks passed
@red-githubbot red-githubbot bot added the Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. label Jun 19, 2023
@Jackenmen Jackenmen modified the milestones: 3.5.4, 3.5.3 Jun 19, 2023
@Jackenmen Jackenmen added Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. and removed Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. labels Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Cogs - Audio This is related to the Audio cog. Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. Type: Enhancement Something meant to enhance existing Red features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants