Skip to content

Conversation

@filzrev
Copy link
Contributor

@filzrev filzrev commented Feb 10, 2024

What included in this PR
Add fail-fast logics when --serve option enabled and specified TCP port is already used.

Background
docfx build commands takes some times to complete.
When combined with --serve options.
Command fails on RunServe::Exec phase.
And need to run the `docfx serve' command separately.

This PR add TCP port availability check before start metadata/build commands.
And fail-fast if port is already used.

Test
I've manually confirmed behaviors with following --hostname patterns

  • null/ localhost
    • It's bound to loopback addressed (both 127.0.0.1 and ::1)
  • *
    • It's bound to all available IPv4/IPv6 addressed
  • IPAddress: (e.g. 127.0.0.1 / 127.0.0.2)
    • I’ve confirmed that docfx can successfully host both sites simultaneously.

@codecov
Copy link

codecov bot commented Feb 10, 2024

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (fe673ec) 74.31% compared to head (6b16be7) 74.25%.
Report is 23 commits behind head on main.

Files Patch % Lines
src/docfx/Models/CommandHelper.cs 0.00% 10 Missing ⚠️
src/docfx/Models/BuildCommand.cs 0.00% 6 Missing ⚠️
src/docfx/Models/DefaultCommand.cs 50.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9690      +/-   ##
==========================================
- Coverage   74.31%   74.25%   -0.07%     
==========================================
  Files         536      536              
  Lines       23189    23214      +25     
  Branches     4056     4064       +8     
==========================================
+ Hits        17234    17237       +3     
- Misses       4853     4874      +21     
- Partials     1102     1103       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yufeih
Copy link
Contributor

yufeih commented Feb 16, 2024

What if we do something like this?

image

@filzrev
Copy link
Contributor Author

filzrev commented Feb 16, 2024

What if we do something like this?

Personally, I prefer the `fail-fast approach' to the proposed approach.
For the following reasons.

  • If BuildCommand is run twice based on the same docfx.config.
    _site contents are overwritten if fail-fast check not exists.
    And the content hosted on `port:8080' will be modified also.
    In this case. it makes no sense to host contents with alternative port.
  • When acceptting user input. It needs additional consideration about non-UserInteractive environment.

Copy link
Contributor

@yufeih yufeih left a comment

Choose a reason for hiding this comment

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

Thank you @filzrev !

@yufeih yufeih merged commit c7346f1 into dotnet:main Feb 18, 2024
@yufeih yufeih added the bug-fix Makes the pull request to appear in "Bug Fixes" section of the next release note label Feb 18, 2024
renovate bot referenced this pull request in dotnet/dotnet-operator-sdk Feb 18, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [docfx](https://togithub.com/dotnet/docfx) | `2.75.2` -> `2.75.3` |
[![age](https://developer.mend.io/api/mc/badges/age/nuget/docfx/2.75.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/docfx/2.75.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/docfx/2.75.2/2.75.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/docfx/2.75.2/2.75.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>dotnet/docfx (docfx)</summary>

### [`v2.75.3`](https://togithub.com/dotnet/docfx/releases/tag/v2.75.3)

<!-- Release notes generated using configuration in .github/release.yml
at main -->

#### What's Changed

##### 🐞 Bug Fixes

- fix: Toc warnings when toc item with TopicUid but name is not
specified by [@&#8203;filzrev](https://togithub.com/filzrev) in
[https://github.com/dotnet/docfx/pull/9665](https://togithub.com/dotnet/docfx/pull/9665)
- fix: Warnings that occurs when bookmark link contains non-ASCII chars
by [@&#8203;filzrev](https://togithub.com/filzrev) in
[https://github.com/dotnet/docfx/pull/9660](https://togithub.com/dotnet/docfx/pull/9660)
- fix: Add StackTrace logs for Javascript error by
[@&#8203;filzrev](https://togithub.com/filzrev) in
[https://github.com/dotnet/docfx/pull/9694](https://togithub.com/dotnet/docfx/pull/9694)
- fix: InvalidInputFile error occurs if file contains URI escaped
charactors by [@&#8203;filzrev](https://togithub.com/filzrev) in
[https://github.com/dotnet/docfx/pull/9700](https://togithub.com/dotnet/docfx/pull/9700)
- fix: Add fail-fast logics when `--serve` option enabled & port is
already used by [@&#8203;filzrev](https://togithub.com/filzrev) in
[https://github.com/dotnet/docfx/pull/9690](https://togithub.com/dotnet/docfx/pull/9690)
- fix: filterconfig exclude rule is not works as documented by
[@&#8203;filzrev](https://togithub.com/filzrev) in
[https://github.com/dotnet/docfx/pull/9666](https://togithub.com/dotnet/docfx/pull/9666)

##### 🔧 Engineering

- build(deps): bump YamlDotNet from 15.1.0 to 15.1.1 dependencies .NET
by [@&#8203;filzrev](https://togithub.com/filzrev) in
[https://github.com/dotnet/docfx/pull/9689](https://togithub.com/dotnet/docfx/pull/9689)
- chore: update NuGet package dependencies
(Microsoft.NET.Test.Sdk,Microsoft.Build.Locator) by
[@&#8203;filzrev](https://togithub.com/filzrev) in
[https://github.com/dotnet/docfx/pull/9693](https://togithub.com/dotnet/docfx/pull/9693)
- chore: Update public API snapshot & disable `AutoVerify` on CI build
by [@&#8203;filzrev](https://togithub.com/filzrev) in
[https://github.com/dotnet/docfx/pull/9692](https://togithub.com/dotnet/docfx/pull/9692)
- chore: Update node.js version by
[@&#8203;filzrev](https://togithub.com/filzrev) in
[https://github.com/dotnet/docfx/pull/9701](https://togithub.com/dotnet/docfx/pull/9701)
- chore: Update Microsoft.Build package version by
[@&#8203;filzrev](https://togithub.com/filzrev) in
[https://github.com/dotnet/docfx/pull/9698](https://togithub.com/dotnet/docfx/pull/9698)

**Full Changelog**:
dotnet/docfx@v2.75.2...v2.75.3

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 9pm,before 6am" in timezone
Europe/Zurich, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

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

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/buehler/dotnet-operator-sdk).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xOTEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjE5MS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
p-kostov pushed a commit to ErpNetDocs/docfx that referenced this pull request Jun 28, 2024
…lready used (dotnet#9690)

chore: Add fail-fast logics when port is already used on serve
@filzrev filzrev deleted the chore-failfast-when-port-already-used branch September 18, 2024 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix Makes the pull request to appear in "Bug Fixes" section of the next release note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants