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

Pick random debug port when the configured one is taken #33548

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

geoand
Copy link
Contributor

@geoand geoand commented May 23, 2023

Closes: #33363

@geoand
Copy link
Contributor Author

geoand commented May 23, 2023

@maxandersen Here you go :)

@quarkus-bot

This comment has been minimized.

@@ -376,14 +376,30 @@ protected void prepare() throws Exception {
if (debug != null && debug.equalsIgnoreCase("client")) {
args.add("-agentlib:jdwp=transport=dt_socket,address=" + debugHost + ":" + port + ",server=n,suspend=" + suspend);
} else if (debug == null || !debug.equalsIgnoreCase("false")) {
// make sure the debug port is not used, we don't want to just fail if something else is using it
// we don't check this on restarts, as the previous process is still running
// if the debug port is used, we want to make an effort to pick another one
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we print a info/warn that is locating a free port because specified one is not working?

btw. do we really need the 5 retries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point about the error message - added.

btw. do we really need the 5 retries?

It's just a detail honestly...

Copy link
Member

Choose a reason for hiding this comment

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

just tested - and yes; we really need to print something about it not using the expected configured port.

Copy link
Member

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

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

works brilliantly but really should print something with error/warning that is not able to use the initial requested port as otherwise you don't notice.

@geoand
Copy link
Contributor Author

geoand commented May 24, 2023

works brilliantly but really should print something with error/warning that is not able to use the initial requested port as otherwise you don't notice.

PR has already been updated :)

@geoand geoand requested a review from maxandersen May 24, 2023 10:20
@quarkus-bot

This comment has been minimized.

@maxandersen
Copy link
Member

PR has already been updated :)

not seeing it in code nor when running?

it prints:

Listening for transport dt_socket at address: 51278

no, info/warning that it had to grab a random port.

Something like:

WARN: Requested port 5005 not available. Trying to find free port.
Listening for transport dt_socket at address: 51278

@geoand
Copy link
Contributor Author

geoand commented May 26, 2023

Ah, you wan't a warning that we had to switch to a random port?

@geoand
Copy link
Contributor Author

geoand commented May 26, 2023

PR updated

@quarkus-bot

This comment has been minimized.

@geoand
Copy link
Contributor Author

geoand commented May 31, 2023

@maxandersen is this good to go?

@maxandersen
Copy link
Member

took 3.1 for a run with quarkus dev -Ddebug=0 and got:

[ERROR] Failed to execute goal io.quarkus.platform:quarkus-maven-plugin:3.1.0.Final:dev (default-cli) on project demo2: Failed to run: The specified debug port must be greater than 0 -> [Help 1]

now trying this PR to see if that is still present.

@maxandersen
Copy link
Member

somehow I thought the debug picking was in 3.1...seems it was not. my bad.

@geoand
Copy link
Contributor Author

geoand commented Jun 4, 2023

We would need to merge this first 😎

@geoand
Copy link
Contributor Author

geoand commented Jun 13, 2023

@maxandersen should we merge this or should I close it?

Copy link
Member

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

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

+1 on this; its awesome.

one minor nit on including port as we did in old messaeg.

@geoand
Copy link
Contributor Author

geoand commented Jun 13, 2023

Suggestion applied

@quarkus-bot

This comment has been minimized.

@geoand
Copy link
Contributor Author

geoand commented Jun 19, 2023

ping :)

@geoand
Copy link
Contributor Author

geoand commented Jun 20, 2023

If we want this in 3.2.0.CR1, we need it merged now

@geoand
Copy link
Contributor Author

geoand commented Oct 3, 2023

I talked to Max and we'll get this in when CI finishes

@geoand geoand dismissed maxandersen’s stale review October 3, 2023 14:13

Suggestions applied

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 3, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 3, 2023

Failing Jobs - Building ba0c261

Status Name Step Failures Logs Raw logs Build scan
JVM Tests - JDK 11 Build Failures Logs Raw logs
✔️ JVM Tests - JDK 17
✔️ JVM Tests - JDK 20
Native Tests - Data7 Build ⚠️ Check → Logs Raw logs

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: extensions/hibernate-orm/deployment 
! Skipped: extensions/flyway/deployment extensions/hibernate-envers/deployment extensions/hibernate-reactive/deployment and 100 more

📦 extensions/hibernate-orm/deployment

Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.1.2:test (default-test) on project quarkus-hibernate-orm-deployment:

Please refer to /home/runner/work/quarkus/quarkus/extensions/hibernate-orm/deployment/target/surefire-reports for the individual test results.
Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
There was an error in the forked process

@geoand
Copy link
Contributor Author

geoand commented Oct 4, 2023

@maxandersen can you approve this? Seems I can't ninja merge :)

Copy link
Member

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

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

big +1

@geoand geoand merged commit 2a2e719 into quarkusio:main Oct 16, 2023
48 of 50 checks passed
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Oct 16, 2023
@quarkus-bot quarkus-bot bot added this to the 3.6 - main milestone Oct 16, 2023
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 16, 2023
@gsmet gsmet modified the milestones: 3.6 - main, 3.5.0 Oct 17, 2023
benkard pushed a commit to benkard/mulkcms2 that referenced this pull request Nov 12, 2023
This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [flow-bin](https://github.com/flowtype/flow-bin) ([changelog](https://github.com/facebook/flow/blob/master/Changelog.md)) | devDependencies | minor | [`^0.219.0` -> `^0.220.0`](https://renovatebot.com/diffs/npm/flow-bin/0.219.0/0.220.0) |
| [org.jsoup:jsoup](https://jsoup.org/) ([source](https://github.com/jhy/jsoup)) | compile | patch | `1.16.1` -> `1.16.2` |
| [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | minor | `3.4.3` -> `3.5.0` |
| [io.quarkus:quarkus-universe-bom](https://github.com/quarkusio/quarkus-platform) | import | minor | `3.4.3` -> `3.5.0` |

---

### Release Notes

<details>
<summary>flowtype/flow-bin</summary>

### [`v0.220.0`](flow/flow-bin@f7f3f3f...030bfc6)

[Compare Source](flow/flow-bin@f7f3f3f...030bfc6)

### [`v0.219.5`](flow/flow-bin@f16a6c7...f7f3f3f)

[Compare Source](flow/flow-bin@f16a6c7...f7f3f3f)

### [`v0.219.4`](flow/flow-bin@9f67075...f16a6c7)

[Compare Source](flow/flow-bin@9f67075...f16a6c7)

### [`v0.219.3`](flow/flow-bin@80dcea5...9f67075)

[Compare Source](flow/flow-bin@80dcea5...9f67075)

### [`v0.219.2`](flow/flow-bin@c184c5d...80dcea5)

[Compare Source](flow/flow-bin@c184c5d...80dcea5)

</details>

<details>
<summary>quarkusio/quarkus</summary>

### [`v3.5.0`](https://github.com/quarkusio/quarkus/releases/tag/3.5.0)

[Compare Source](quarkusio/quarkus@3.4.3...3.5.0)

##### Complete changelog

-   [#&#8203;36527](quarkusio/quarkus#36527) - Start MongoDB 4.4 instead of 4.0
-   [#&#8203;36523](quarkusio/quarkus#36523) - Minor OIDC Auth0 updates
-   [#&#8203;36518](quarkusio/quarkus#36518) - Allow for setting logging scope programmatically
-   [#&#8203;36517](quarkusio/quarkus#36517) - Use Mandrel 23.1 in windows CI
-   [#&#8203;36501](quarkusio/quarkus#36501) - Let custom OIDC token propagation filters customize the exchange status
-   [#&#8203;36495](quarkusio/quarkus#36495) - Support external OTel exporters in CDI
-   [#&#8203;36490](quarkusio/quarkus#36490) - Take ReaderInterceptor into account when reading SSE events
-   [#&#8203;36487](quarkusio/quarkus#36487) - Upgrade to Liquibase 4.24.0
-   [#&#8203;36485](quarkusio/quarkus#36485) - Fix typo in gradle-tooling.adoc
-   [#&#8203;36474](quarkusio/quarkus#36474) - Fix some issues in getting-started-dev-services
-   [#&#8203;36465](quarkusio/quarkus#36465) - Be more consistent in guides when creating projects/adding extensions
-   [#&#8203;36464](quarkusio/quarkus#36464) - HTTP reference guide - HTTP/2 section update, drop JDK 8 note
-   [#&#8203;36459](quarkusio/quarkus#36459) - Let custom OIDC token propagation filters provide client name
-   [#&#8203;36457](quarkusio/quarkus#36457) - Update builder images to jdk-21
-   [#&#8203;36453](quarkusio/quarkus#36453) - Upgrade Oracle JDBC driver to 23.3.0.23.09
-   [#&#8203;36452](quarkusio/quarkus#36452) - Fix doc extension-add.adoc
-   [#&#8203;36451](quarkusio/quarkus#36451) - Adjust extension name for consistency with rest of Quarkus
-   [#&#8203;36446](quarkusio/quarkus#36446) - Regression: Liquibase fails to migrate on Quarkus start, crashing the application
-   [#&#8203;36445](quarkusio/quarkus#36445) - Updates to Infinispan 14.0.19.Final
-   [#&#8203;36442](quarkusio/quarkus#36442) - Use default content type when X-SSE header not set
-   [#&#8203;36436](quarkusio/quarkus#36436) - Upgrade to Hibernate ORM 6.2.13.Final
-   [#&#8203;36432](quarkusio/quarkus#36432) - Hibernate Reactive Panache: improve error message
-   [#&#8203;36420](quarkusio/quarkus#36420) - Allow parallel execution of blocking health checks
-   [#&#8203;36419](quarkusio/quarkus#36419) - Blocking Health Checks should be executed in parallel, not sequentially/ordered
-   [#&#8203;36417](quarkusio/quarkus#36417) - Reduce timeout of the doc build to 60 minutes
-   [#&#8203;36413](quarkusio/quarkus#36413) - Simplify virtual threads guide by pushing users to 21
-   [#&#8203;36412](quarkusio/quarkus#36412) - Drop Optaplanner from the documentation
-   [#&#8203;36411](quarkusio/quarkus#36411) - Drop panache topic from Hibernate Reactive guide
-   [#&#8203;36410](quarkusio/quarkus#36410) - Add compatibility topic to Spring guides
-   [#&#8203;36407](quarkusio/quarkus#36407) - Register RuntimeOverrideConfigSource in STATIC_INIT
-   [#&#8203;36406](quarkusio/quarkus#36406) - AssembleDownstreamDocumentation - print guide name
-   [#&#8203;36400](quarkusio/quarkus#36400) - Add topics and extensions metadata to guides
-   [#&#8203;36367](quarkusio/quarkus#36367) - Bump org.wiremock:wiremock-standalone from 3.1.0 to 3.2.0
-   [#&#8203;36365](quarkusio/quarkus#36365) - Bump de.flapdoodle.embed:de.flapdoodle.embed.mongo from 4.7.0 to 4.9.2
-   [#&#8203;36360](quarkusio/quarkus#36360) - Drop the old Dev UI guide
-   [#&#8203;36337](quarkusio/quarkus#36337) - Upgrade maven to version 3.9.5
-   [#&#8203;36236](quarkusio/quarkus#36236) - No Panache session in REST endpoints defined by an interface
-   [#&#8203;35931](quarkusio/quarkus#35931) - Add OIDC Auth0 extended tutorial
-   [#&#8203;33548](quarkusio/quarkus#33548) - Pick random debug port when the configured one is taken
-   [#&#8203;33363](quarkusio/quarkus#33363) - allow quarkus dev to pick random debug port

</details>

<details>
<summary>quarkusio/quarkus-platform</summary>

### [`v3.5.0`](quarkusio/quarkus-platform@3.4.3...3.5.0)

[Compare Source](quarkusio/quarkus-platform@3.4.3...3.5.0)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

👻 **Immortal**: This MR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

allow quarkus dev to pick random debug port
3 participants