-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 namespaceDefinitionType to standardSync #3813
Conversation
ed5bb6c
to
13b34e0
Compare
|
||
// This migration does the following: | ||
// 1. Add fields to StandardSync. | ||
public class MigrationV0_25_0 extends BaseMigration implements Migration { |
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.
you preempted my migration question!
@@ -2096,6 +2102,13 @@ components: | |||
- days | |||
- weeks | |||
- months | |||
NamespaceDefinitionType: |
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.
discussed offline: we should leave a comment here explaining the default behaviour
return catalog; | ||
} | ||
|
||
@Override | ||
public AirbyteMessage mapMessage(final AirbyteMessage inputMessage) { | ||
if (inputMessage.getType() == Type.RECORD) { | ||
final AirbyteMessage message = Jsons.clone(inputMessage); | ||
if (namespaceDefinition != null && namespaceDefinition.equals(NamespaceDefinitionType.DESTINATION)) { |
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.
spoke offline: same comment here about default behaviour possibly linking to the api spec file.
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.
Looks great!
One comment about better explanation around the default behaviour. Feel free to merge whenever.
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.
looks good!
What
Implements backend part of #3481
How
Expose an option on how to use the source namespaces in the destination:
Pre-merge Checklist
Recommended reading order
airbyte-workers/src/main/java/io/airbyte/workers/protocols/airbyte/NamespacingMapper.java
airbyte-api/src/main/openapi/config.yaml
see https://docs.google.com/document/d/1qFk4YqnwxE4MCGeJ9M2scGOYej6JnDy9A0zbICP_zjI/edit# for more context