Skip to content

Pass timeout in executioncontext instead of stepsrunner - #2714

Merged
fhammerl merged 2 commits into
mainfrom
fhammerl/timeout-composite-fix
Jul 25, 2023
Merged

Pass timeout in executioncontext instead of stepsrunner#2714
fhammerl merged 2 commits into
mainfrom
fhammerl/timeout-composite-fix

Conversation

@fhammerl

Copy link
Copy Markdown
Contributor

No description provided.

@fhammerl
fhammerl requested a review from a team as a code owner July 25, 2023 09:00
Trace.Info($"Starting: {step.DisplayName}");
step.ExecutionContext.Debug($"Starting: {step.DisplayName}");
// composite steps inherit the timeout from the parent, set by https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepstimeout-minutes
step.ExecutionContext.SetTimeout(step.ExecutionContext.Parent.GetRemainingTimeout());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In previous fix I treated step.ExecutionContext.Parent.ExecutionContext as the EC of workflow or composite step creating step, but it is actually the EC of the job because comp steps are created by Root

nikola-jokic
nikola-jokic previously approved these changes Jul 25, 2023
JoannaaKL
JoannaaKL previously approved these changes Jul 25, 2023
@fhammerl
fhammerl dismissed stale reviews from JoannaaKL and nikola-jokic via b8db400 July 25, 2023 11:54
@fhammerl
fhammerl merged commit 7cb6192 into main Jul 25, 2023
@fhammerl
fhammerl deleted the fhammerl/timeout-composite-fix branch July 25, 2023 12:12
fhammerl added a commit that referenced this pull request Jul 25, 2023
* Pass timeout in executioncontext instead of stepsrunner

* Fix linting
fhammerl added a commit that referenced this pull request Jul 25, 2023
* Pass timeout in executioncontext instead of stepsrunner

* Fix linting
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