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

[automation/nodejs] - Expose structured logging #6454

Merged
merged 13 commits into from
Mar 11, 2021

Conversation

komalali
Copy link
Member

@komalali komalali commented Mar 2, 2021

See also: #6436 for the go implementation.

Changes

  • Adds an onEvent option to Stack.up(), Stack.preview(), Stack.refresh() and Stack.destroy() for acting on structured json events.
  • Updates the PreviewResult interface to include a changeSummary property that summarizes changes to resources.

Fixes: #6442

@komalali komalali force-pushed the komalali/automation-api-events branch from daff9f6 to 9392c9f Compare March 4, 2021 04:56
@komalali komalali changed the title Expose structured event stream to JavaScript Automation API [automation/nodejs] - Expose structured logging Mar 4, 2021
@komalali komalali force-pushed the komalali/automation-api-events branch 2 times, most recently from b364c1e to 86da460 Compare March 9, 2021 02:17
@komalali komalali force-pushed the komalali/automation-api-events branch from 86da460 to 397a52b Compare March 9, 2021 21:52
@komalali komalali marked this pull request as ready for review March 9, 2021 21:57
Copy link
Member

@lukehoban lukehoban left a comment

Choose a reason for hiding this comment

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

Looks great!

// Op is the operation being performed, a deploy.StepOp.
op: string;
urn: string;
type: string;
Copy link
Member

Choose a reason for hiding this comment

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

This may have been my doing in the original draft PR - but we likely want to make sure all the properties are documented here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I went through and did a second pass, the ones still missing don't have a description in common/apitype/events.

sdk/nodejs/x/automation/stack.ts Outdated Show resolved Hide resolved
sdk/nodejs/x/automation/stack.ts Outdated Show resolved Hide resolved
sdk/nodejs/x/automation/stack.ts Outdated Show resolved Hide resolved
sdk/nodejs/x/automation/stack.ts Outdated Show resolved Hide resolved
sdk/nodejs/x/automation/stack.ts Outdated Show resolved Hide resolved
sdk/nodejs/x/automation/stack.ts Show resolved Hide resolved
sdk/nodejs/x/automation/stack.ts Outdated Show resolved Hide resolved
sdk/nodejs/x/automation/stack.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@EvanBoyle EvanBoyle left a comment

Choose a reason for hiding this comment

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

👍 🔥

sdk/nodejs/x/automation/events.ts Outdated Show resolved Hide resolved
@komalali komalali force-pushed the komalali/automation-api-events branch from 6ecc281 to cf202c9 Compare March 11, 2021 18:25
@komalali komalali merged commit 1fc2ba4 into master Mar 11, 2021
@komalali komalali deleted the komalali/automation-api-events branch March 11, 2021 19:45
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.

[automation/nodejs] - Add structured output to preview/up/refresh/destroy
3 participants