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

Add typed transform inputs #175

Merged
merged 16 commits into from
Jan 25, 2023
Merged

Add typed transform inputs #175

merged 16 commits into from
Jan 25, 2023

Conversation

narape
Copy link
Contributor

@narape narape commented Jan 23, 2023

TL;DR

This feature allows the users to know the specific inputs from a task at development time. This is really important when you are working with a remote task that is developed by other users.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

This feature allows the users to know the specific inputs from a task at development time. This is really important when you are working with a remote task that is developed by other users. The final goal is to use the input class in the WorkflowBuilder instead of passing the input into the SdkTransform using the withInput(...) method that forces you to know the attribute names of the input class. Example: Fibonacci Workflow

Tracking Issue

Follow-up issue

NA

@narape narape force-pushed the typed-inputs branch 3 times, most recently from 39b162e to 517f129 Compare January 24, 2023 07:54
narape and others added 6 commits January 24, 2023 09:48
... flytekit-java compiles but tests fails

Signed-off-by: Nelson Arapé <[email protected]>
Signed-off-by: Nelson Arapé <[email protected]>
Signed-off-by: Andres Gomez Ferrer <[email protected]>
Signed-off-by: Nelson Arapé <[email protected]>
Signed-off-by: Nelson Arapé <[email protected]>
Signed-off-by: Nelson Arapé <[email protected]>
Signed-off-by: Nelson Arapé <[email protected]>
Signed-off-by: Nelson Arapé <[email protected]>
andresgomezfrr and others added 6 commits January 24, 2023 12:33
@andresgomezfrr andresgomezfrr changed the title Typed inputs Add typed transform inputs Jan 24, 2023
@andresgomezfrr andresgomezfrr marked this pull request as ready for review January 24, 2023 14:56
/**
* Generate an immutable value class that represents {@link AddQuestionTask}'s input, which is a
* String.
*/
@AutoValue
public abstract static class Input {
public abstract SdkBindingData<String> greeting();

public static Input create(SdkBindingData<String> greeting) {
Copy link
Contributor Author

@narape narape Jan 24, 2023

Choose a reason for hiding this comment

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

Note to reviewers

Before we were only using the AutoValue to define the type of the task, but now we actually create the auto values to pass it in the apply

import javax.annotation.Nullable;

public class SdkCondition<OutputT> extends SdkTransform<OutputT> {
public class SdkCondition<OutputT> extends SdkTransform<Void, OutputT> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for reviewers

We made a SdkCondition to have no inputs on itself but the transformations going on when, and otherwise cases could have a combination of both:

  • SdkTransforms that takes no inputs
  • SdkTransforms that takes inputs but the inputs are supplied

All of them should have the same ouput type

@@ -91,4 +74,16 @@ public SdkTransform<T> withTimeoutOverride(Duration timeout) {
public String getName() {
return getClass().getName();
}

void checkNullOnlyVoid(@Nullable InputT inputs) {
Copy link
Contributor Author

@narape narape Jan 24, 2023

Choose a reason for hiding this comment

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

Note to reviewers

This method makes sure that the input "nullness" matches the input type:

  • null in the only valid value for Void and Unit SdkTypes
  • Any other SdkType cannot have null

If you have a better name please suggest it

Signed-off-by: Nelson Arapé <[email protected]>
@andresgomezfrr andresgomezfrr mentioned this pull request Jan 25, 2023
8 tasks
@andresgomezfrr andresgomezfrr merged commit 2a40a16 into master Jan 25, 2023
@andresgomezfrr andresgomezfrr deleted the typed-inputs branch January 25, 2023 10:33
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