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 support to generate random management port #8023

Closed
wants to merge 1 commit into from

Conversation

eddumelendez
Copy link
Contributor

Previous to this commit, in order to provide a random port for
management key property should be explicitly provided. Now,
SpringBootTest annotation can do this if
WebEnvironment.RANDOM_SERVER_AND_MANAGEMENT_PORT is set.

@RunWith(SpringRunner.class)
@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_SERVER_AND_MANAGEMENT_PORT)
public classSpringBootApplicationTests {

}

See gh-4424

Previous to this commit, in order to provide a random port for
management key property should be explicitly provided. Now,
SpringBootTest annotation can do this if
WebEnvironment.RANDOM_SERVER_AND_MANAGEMENT_PORT is set.

```
@RunWith(SpringRunner.class)
@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_SERVER_AND_MANAGEMENT_PORT)
public classSpringBootApplicationTests {

}
```

See spring-projectsgh-4424
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 19, 2017
@philwebb philwebb added priority: normal type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 19, 2017
@philwebb philwebb added this to the 1.5.0 milestone Jan 19, 2017
@snicoll
Copy link
Member

snicoll commented Jan 24, 2017

Thanks for the PR eddu!

@philwebb I've followed the discussion in the related issue and I am not super happy about the new enum value. I think we should fix RANDOM_PORT so that it uses a random port for the actuator port if a dedicated port has been requested via configuration. That way we would leave the same port (99% of the use case) but would happily change it in case a different one was requested. This would probably require us to register an EnvironmentPostProcessor of some kind in the test support first.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Jan 24, 2017
@philwebb philwebb removed this from the 1.5.0 milestone Jan 25, 2017
@philwebb
Copy link
Member

I see your point @snicoll. Did you mean "it uses a random port for the actuator port if a dedicated port has not been requested via configuration"?

Either way, let's not rush this into 1.5 if we're not 100% sure.

@philwebb philwebb added the status: on-hold We can't start working on this issue yet label Jan 25, 2017
@snicoll
Copy link
Member

snicoll commented Jan 25, 2017

No. The reason why we don't flip management.port=0 with RANDOM_PORT is because if the user hasn't request one via config, we would start on a different port. So we need to be able to detect that case and add the random port only if that's the case. If the user hasn't requested anything, the regular port will be used for both use cases and we'll be fine.

@philwebb philwebb removed for: team-attention An issue we'd like other members of the team to review status: on-hold We can't start working on this issue yet labels Feb 1, 2017
@philwebb
Copy link
Member

philwebb commented Feb 1, 2017

Thanks @eddumelendez, given @snicoll's comments we'll try a different approach.

@philwebb philwebb closed this Feb 1, 2017
@wilkinsona wilkinsona added the status: declined A suggestion or change that we don't feel we should currently apply label May 14, 2017
@eddumelendez eddumelendez deleted the gh-4424 branch January 17, 2018 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants