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

feat: add an initial set of execution steps #3214

Merged
merged 1 commit into from
Aug 15, 2019

Conversation

rodesai
Copy link
Contributor

@rodesai rodesai commented Aug 14, 2019

This patch adds an initial set of execution step nodes. Subsequent patches
will have schemakstream/table build the execution tree as it goes along, and
then we'll move calls to streams into the implementations of ExecutionStep.build.

All execution steps implement ExecutionStep, which along with supporting a few
common properties (step id and schema), includes a method called build(), which
will eventually get called to build the streams app (as described above).

This patch adds an initial set of execution step nodes. Subsequent patches
will have schemakstream/table build the execution tree as it goes along, and
then we'll move calls to streams into the implementations of ExecutionStep.build.

All execution steps implement ExecutionStep, which along with supporting a few
common properties (step id and schema), includes a method called build(), which
will eventually get called to build the streams app (as described above).
@rodesai rodesai requested a review from a team as a code owner August 14, 2019 18:37
@agavra
Copy link
Contributor

agavra commented Aug 14, 2019

Don't all of the ExecutionStep classes need to be serializable somehow? Or will that be addressed in a future patch? Otherwise this LGTM - nice to have this PR introduce just boilerplate.

@rodesai
Copy link
Contributor Author

rodesai commented Aug 15, 2019

Don't all of the ExecutionStep classes need to be serializable somehow? Or will that be addressed in a future patch? Otherwise this LGTM - nice to have this PR introduce just boilerplate.

@agavra yes this (along with patches to serialize expressions) is coming. I wanted to define the steps with this PR, then start submitting PRs to:
- build the plan from (Grouped)SchemaKStream|Table
- move building of the streams app into the step classes
- serialize the steps (and expressions) to the command/config topic for interactive/standalone mode.

Copy link
Contributor

@agavra agavra left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the clarifications on next steps @rodesai

@agavra agavra requested a review from a team August 15, 2019 15:37
@rodesai rodesai merged commit c860793 into confluentinc:master Aug 15, 2019
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