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

[rush] Exposing executionRecords in PhasedScriptsAction allows users to observe the execution process #4010

Merged

Conversation

sherlockfeng
Copy link
Contributor

@sherlockfeng sherlockfeng commented Mar 14, 2023

Summary

Exposing executionRecords in PhasedScriptsAction allows users to observe the execution process

Details

As a member of the monorepo infrastructure team, there will be many compilations in progress at the same time,
I hope to be able to collect the real-time status of the rush execution command. If I can get the real-time status in the hooks of rushLifeCycle, I can observe the start and end times of all project compilation in a task, and then send it to the data server to build a dashboard that can view the execution status in real time

How to use

rushSession.hooks.runAnyPhasedCommand.tap(PLUGIN_NAME, (phase: ReadonlyMap<Operation, IOperationExecutionResult>) => {
    phase.hooks.beforeExecuteOperations.tapPromise(PLUGIN_NAME, async (records) => {
        // all operation records
    });
    phase.hooks.onOperationStatusChanged.tap(PLUGIN_NAME, (record: IOperationExecutionResult) => {
        // statue changed operation record
    });
});

Demo use local data

image

@dmichon-msft
Copy link
Contributor

I needed a way to expose this data in #3651 as well, and at the time it seemed more useful to add an additional hook to PhasedOperationHooks that gets invoked whenever the state of an individual operation changes, rather than having to poll the execution. See these callbacks here: https://github.com/microsoft/rushstack/pull/3651/files#diff-454747232f90e3a30fe693116f4913b6ca94cdbe7e9eb5b4a94e744b69fc6c3dR25-R26

I'd prefer not to add additional API surface to the PhasedScriptAction class and have any extensibility mechanisms be via the PhasedOperationHooks object for consistency.

@sherlockfeng sherlockfeng force-pushed the feat/periodically-send-operation-state branch from 861f1d1 to 9ca56d6 Compare March 17, 2023 02:52
@sherlockfeng sherlockfeng force-pushed the feat/periodically-send-operation-state branch 4 times, most recently from a559bd5 to 4d21375 Compare March 17, 2023 06:56
Copy link
Contributor

@dmichon-msft dmichon-msft left a comment

Choose a reason for hiding this comment

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

Let's avoid floating promise issues. Either await async hooks or use sync hooks, depending on if it makes sense for the given hook to potentially modify state.

Edit: otherwise, looks about ready.

@sherlockfeng sherlockfeng force-pushed the feat/periodically-send-operation-state branch from 6550166 to 5d05d6e Compare March 18, 2023 03:57
@sherlockfeng sherlockfeng force-pushed the feat/periodically-send-operation-state branch from 5d05d6e to d58485c Compare March 18, 2023 03:59
Copy link
Contributor

@dmichon-msft dmichon-msft left a comment

Choose a reason for hiding this comment

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

LGTM

Co-authored-by: David Michon <[email protected]>
@iclanton iclanton enabled auto-merge March 20, 2023 18:30
@iclanton iclanton merged commit 7e48a95 into microsoft:main Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants