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

React to ANCM requestTimeout change for 2.1 #5949

Merged
merged 4 commits into from
Apr 13, 2018
Merged

Conversation

guardrex
Copy link
Collaborator

@guardrex guardrex commented Apr 12, 2018

Fixes #5264

Internal Review Topic

In the <=2.0 and >=2.1 rendered table content, you only need to look at the requestTimeout row Description column content.

@guardrex guardrex requested a review from jkotalik April 12, 2018 00:47
| `shutdownTimeLimit` | <p>Optional integer attribute.</p><p>Duration in seconds that the module waits for the executable to gracefully shutdown when the *app_offline.htm* file is detected.</p> | `10` |
| `startupTimeLimit` | <p>Optional integer attribute.</p><p>Duration in seconds that the module waits for the executable to start a process listening on the port. If this time limit is exceeded, the module kills the process. The module attempts to relaunch the process when it receives a new request and continues to attempt to restart the process on subsequent incoming requests unless the app fails to start **rapidFailsPerMinute** number of times in the last rolling minute.</p> | `120` |
| `stdoutLogEnabled` | <p>Optional Boolean attribute.</p><p>If true, **stdout** and **stderr** for the process specified in **processPath** are redirected to the file specified in **stdoutLogFile**.</p> | `false` |
| `stdoutLogFile` | <p>Optional string attribute.</p><p>Specifies the relative or absolute file path for which **stdout** and **stderr** from the process specified in **processPath** are logged. Relative paths are relative to the root of the site. Any path starting with `.` are relative to the site root and all other paths are treated as absolute paths. Any folders provided in the path must exist in order for the module to create the log file. Using underscore delimiters, a timestamp, process ID, and file extension (*.log*) are added to the last segment of the **stdoutLogFile** path. If `.\logs\stdout` is supplied as a value, an example stdout log is saved as *stdout_20180205194132_1934.log* in the *logs* folder when saved on 2/5/2018 at 19:41:32 with a process ID of 1934.</p> | `aspnetcore-stdout` |
::: moniker-end
::: moniker range=">= aspnetcore-2.1"
[!INCLUDE[](~/includes/2.1.md)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Any way to minimize the repetition here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How do you mean ... the moniker ranges ... no can do unfortunately. They don't work inside table cells AFAIK.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah that's what I meant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Their custom bits (e.g., tab controls, markdown lists, code blocks) blow up 💥 inside table cells. 😢 Some of it we can hack, but this is one that I don't think we can work around.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Rick-Anderson @scottaddie Do you have any info? ... we can't break out by table row, can we?

Copy link
Contributor

Choose a reason for hiding this comment

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

::: moniker range="moniker-range-expression"

Versioned Content

::: moniker-end

A versioned zones starts with ::: moniker on a new line and specifies a range property, the value of which is a valid moniker range. The zone ends with ::: moniker-end, on its own line. All content between these two markers applies to the versions matched by the range expression.

Limitations of Zones

  1. You can use includes inside of a zone, along with any other standard Markdown or Docs-specific Markdown extensions.
  2. You can use a zone inside of other constructs, like a blockquote or note.
  3. You cannot use a moniker zone inside of another moniker zone. Nesting is not permitted.

Copy link
Collaborator Author

@guardrex guardrex Apr 12, 2018

Choose a reason for hiding this comment

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

... so they don't really say explicitly if we can inject them into a table. Has it been tested? ... I can test it on this PR if you want.

| Col1 | Col2 |
| ---- | ---- |
| Val1 | Val2 |
::: moniker range="<= aspnetcore-1.1"
| Val1 | Val2 |
::: moniker-end
::: moniker range=">= aspnetcore-2.0"
| Val1 | Val2 |
::: moniker-end
| Val1 | Val2 |

Copy link
Contributor

Choose a reason for hiding this comment

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

@guardrex try that. Currently the entire table is missing when I select 2.0

@guardrex guardrex requested a review from Rick-Anderson April 12, 2018 05:17
@guardrex
Copy link
Collaborator Author

guardrex commented Apr 12, 2018

@Rick-Anderson Try it now (that didn't throw a build error) ... Internal Review Topic ... it's the requestTimeout row, and you should see the preview notice, too.

WRT it not showing for 2.0 before ... rookie first-timer mistake. I know why it was broken; so if we end up going back to my original two-table approach, it should work.

@Rick-Anderson
Copy link
Contributor

@guardrex breaks table rendering. I think I'll squash/merge and open a bug.

image

@Rick-Anderson
Copy link
Contributor

@guardrex can you move he requestTimeout row to the bottom of the table? You could have a he requestTimeout row that said, see the requestTimeout row at the bottom of the table.

@Rick-Anderson
Copy link
Contributor

@guardrex I think it's OK that it breaks the table rendering for now - especially if you move it to the bottom of the table.

@guardrex
Copy link
Collaborator Author

I'd prefer not to do it based on aesthetics. Why not just two tables (I fixed the earlier 2.0 problem)? Internal Review Topic

@Rick-Anderson
Copy link
Contributor

I'm thinking they can fix the bug fairly quickly. If not, use 2 tables.

@guardrex
Copy link
Collaborator Author

guardrex commented Apr 12, 2018

... also my table is alphabetical. It would upset the world order ... Earth may fly out of its orbit into the Sun!

I wouldn't want the deaths of billions of humans on my hands. 😀

@guardrex
Copy link
Collaborator Author

I reverted it ... see if it works now.

| `shutdownTimeLimit` | <p>Optional integer attribute.</p><p>Duration in seconds that the module waits for the executable to gracefully shutdown when the *app_offline.htm* file is detected.</p> | `10` |
| `startupTimeLimit` | <p>Optional integer attribute.</p><p>Duration in seconds that the module waits for the executable to start a process listening on the port. If this time limit is exceeded, the module kills the process. The module attempts to relaunch the process when it receives a new request and continues to attempt to restart the process on subsequent incoming requests unless the app fails to start **rapidFailsPerMinute** number of times in the last rolling minute.</p> | `120` |
| `stdoutLogEnabled` | <p>Optional Boolean attribute.</p><p>If true, **stdout** and **stderr** for the process specified in **processPath** are redirected to the file specified in **stdoutLogFile**.</p> | `false` |
| `stdoutLogFile` | <p>Optional string attribute.</p><p>Specifies the relative or absolute file path for which **stdout** and **stderr** from the process specified in **processPath** are logged. Relative paths are relative to the root of the site. Any path starting with `.` are relative to the site root and all other paths are treated as absolute paths. Any folders provided in the path must exist in order for the module to create the log file. Using underscore delimiters, a timestamp, process ID, and file extension (*.log*) are added to the last segment of the **stdoutLogFile** path. If `.\logs\stdout` is supplied as a value, an example stdout log is saved as *stdout_20180205194132_1934.log* in the *logs* folder when saved on 2/5/2018 at 19:41:32 with a process ID of 1934.</p> | `aspnetcore-stdout` |
::: moniker-end
::: moniker range=">= aspnetcore-2.1"
[!INCLUDE[](~/includes/2.1.md)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove [!INCLUDE] ? It's confusing at the top of the table.

@guardrex guardrex merged commit d83a16d into master Apr 13, 2018
@guardrex guardrex deleted the guardrex/ancm-update branch April 13, 2018 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants