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

move requires_typed_event_stream and type_event_stream_error_message to op context #18521

Closed
wants to merge 1 commit into from

Conversation

jamiedemaria
Copy link
Contributor

@jamiedemaria jamiedemaria commented Dec 5, 2023

Summary & Motivation

Moves required_typed_event_stream and type_event_stream_error_message and associated properties and functions to OpExecutionContext instead of StepContext. This will help later up in the stack for making the OpExecutionContext and RunlessOpExecutionContext more similar

How I Tested These Changes

Copy link
Collaborator

@smackesey smackesey left a comment

Choose a reason for hiding this comment

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

For the record, the reason requires_typed_event_stream was on StepExecutionContext is that at the time it was added, it was used in places where there was no access to OpExecutionContext. That's apparently no longer true, so it can be moved down.

@jamiedemaria
Copy link
Contributor Author

closing, since moving these isn't required for the basic changes to get the context stack merged. We may need to do this move in the future when the asset context is no longer a subclass of op context, but we will figure that out at a later date

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.

2 participants