-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
update api for per stream #13835
update api for per stream #13835
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
import io.airbyte.api.model.generated.ConnectionReadList; | ||
import io.airbyte.api.model.generated.ConnectionSearch; | ||
import io.airbyte.api.model.generated.ConnectionState; | ||
import io.airbyte.api.model.generated.ConnectionStateType; | ||
import io.airbyte.api.model.generated.ConnectionUpdate; | ||
import io.airbyte.api.model.generated.CustomDestinationDefinitionCreate; | ||
import io.airbyte.api.model.generated.CustomDestinationDefinitionUpdate; | ||
|
@@ -727,6 +728,11 @@ public ConnectionState getState(final ConnectionIdRequestBody connectionIdReques | |
return execute(() -> schedulerHandler.getState(connectionIdRequestBody)); | ||
} | ||
|
||
@Override | ||
public ConnectionStateType getStateType(final ConnectionIdRequestBody connectionIdRequestBody) { | ||
return null; | ||
} | ||
Comment on lines
+732
to
+734
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just checking - do we have an issue for implementing this endpoint specifically? Looking at the whimsical board, I only see issues for implementing the CatalogDiff behavior There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was bundling it in with this issue #13648. |
||
|
||
// SCHEDULER | ||
@Override | ||
public CheckConnectionRead executeSourceCheckConnection(final SourceCoreConfig sourceConfig) { | ||
|
@@ -849,7 +855,7 @@ public boolean canImportDefinitons() { | |
return archiveHandler.canImportDefinitions(); | ||
} | ||
|
||
private <T> T execute(final HandlerCall<T> call) { | ||
private static <T> T execute(final HandlerCall<T> call) { | ||
try { | ||
return call.call(); | ||
} catch (final ConfigNotFoundException e) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random thought - should the CatalogDiff contain other changes to streams besides just additions, removals, and field schema changes?
For example, should we be communicating changes to any of the following fields for a stream in the CatalogDiff?
My guess is that we aren't putting these changes in the diff because the diff is just meant to communicate schema changes, but I wanted to double check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great point! in the future, yes. i don't think we need it at this state. that's why i structure this as a list with each item having a type instead of a list per type. i think the number of types will grow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense! Yeah definitely not worth adding now if we don't need it at the moment, and this seems like an extensible approach that will allow us to add more stream transformation types in the future as we need them 👍