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.

Sign up for GitHub

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

Jump to bottom

Update PortAssigner javadoc to better explain behavior #428

Closed
sleberknight opened this issue Jan 11, 2024 · 0 comments · Fixed by #429
Closed

Update PortAssigner javadoc to better explain behavior #428

sleberknight opened this issue Jan 11, 2024 · 0 comments · Fixed by #429
Assignees
@sleberknight
Labels
documentation Improvements or additions to documentation
Milestone
3.4.0

Comments

@sleberknight
Copy link
Member

Update/enhance PortAssigner#assignDynamicPorts javadoc to better explain its behavior.

Specifically:

  • When using HTTP, only the first application and admin connectors have dynamic ports assigned. If an application is using multiple connectors, it's probably not a good idea to use PortAssigner at all.
  • When using HTTPS, currently it replaces any existing application and admin connectors. This is currently in an impleNote but I think it needs to be more prominent until and unless PortAssigner replaces app/admin connectors #427 is implemented to change the behavior. Even if it is changed, like HTTP it would only modify the first application and admin connectors, so it probably should not be used if an application uses multiple connectors.
@sleberknight sleberknight added the documentation Improvements or additions to documentation label Jan 11, 2024
@sleberknight sleberknight added this to the 3.4.0 milestone Jan 11, 2024
sleberknight added a commit that referenced this issue Jan 17, 2024
@sleberknight
Improve PortAssigner#assignDynamicPorts javadocs
af7e7fb 
* Add more details to the javadoc to better explain the
  differences in behavior when using HTTP vs HTTPS ports.
* Emphasize the word 'replace' in a debugging log message.
* Make the TestConfig nested class public inside
  ConfigResourceTest, because IntelliJ flagged this as
  a case where the class using it (TestApp) was public
  and had more visibility than a class it was using.
  While this didn't cause the test any problems, better
  to listen to IntelliJ when it tells you something!
  And it's correct that normally, this would have
  been an issue in production code.

Closes #428
@sleberknight sleberknight self-assigned this Jan 17, 2024
@sleberknight sleberknight mentioned this issue Jan 17, 2024
Merged
sleberknight added a commit that referenced this issue Jan 17, 2024
@sleberknight
Improve PortAssigner#assignDynamicPorts javadocs (#429)
7927d8c 
* Add more details to the javadoc to better explain the
  differences in behavior when using HTTP vs HTTPS ports.
* Emphasize the word 'replace' in a debugging log message.
* Make the TestConfig nested class public inside
  ConfigResourceTest, because IntelliJ flagged this as
  a case where the class using it (TestApp) was public
  and had more visibility than a class it was using.
  While this didn't cause the test any problems, better
  to listen to IntelliJ when it tells you something!
  And it's correct that normally, this would have
  been an issue in production code.

Closes #428
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sleberknight sleberknight

Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
3.4.0
Development

Successfully merging a pull request may close this issue.

Improve PortAssigner#assignDynamicPorts javadocs kiwiproject/dropwizard-service-utilities
1 participant
@sleberknight