🎉 Source Mailchimp: updated Mailchimp schemas #7975
Conversation
|
vmaltsev seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
# Conflicts: # airbyte-config/init/src/main/resources/seed/source_specs.yaml
|
/test connector=connectors/destination-s3
|
VitaliiMaltsev
changed the title
| final Schema singleFieldSchema = getSingleFieldType(combined | ||
| // avro schema allows only unique names of records in case of union | ||
| ? fieldName.concat("_").concat(String.valueOf(definition.getIndex())) | ||
| : fieldName, type, definition.getValue()); |
There was a problem hiding this comment.
We can extract to separate method getCombinedFieldName(combined, fieldName, definition.getIndex()) for better reading
| .flatMap(type -> { | ||
| final Schema singleFieldSchema = getSingleFieldType(combined | ||
| // avro schema allows only unique names of records in case of union | ||
| ? fieldName.concat("_").concat(String.valueOf(definition.getIndex())) |
There was a problem hiding this comment.
Under which circumstance will the names have conflict here? The stream calls distinct at the end, so any redundant types will be removed.
Is the change here related to $ref? Currently the COMBINED type is only used for processing combined restrictions, i.e. oneOf, allOf, and anyOf. I don't think it can or should be be used to handle $ref.
There was a problem hiding this comment.
Under which circumstance will the names have conflict here? The stream calls
distinctat the end, so any redundant types will be removed.Is the change here related to
$ref? Currently theCOMBINEDtype is only used for processing combined restrictions, i.e.oneOf,allOf, andanyOf. I don't think it can or should be be used to handle$ref.
i find this behaviour when converting objects from this schema (Mailchimp)
{
"type": "array",
"title": "Segment Type",
"description": "Segment match conditions. There are multiple possible types, see the [condition types documentation](https://mailchimp.com/developer/marketing/docs/alternative-schemas/#segment-condition-schemas).",
"items": {
"x-discriminator": {
"type": "string",
"propertyName": "condition_type"
},
"oneOf": [
{
"$ref": "aimSegment.json"
},
{
"$ref": "automationSegment.json"
},
{
"$ref": "campaignPollSegment.json"
},
{
"$ref": "conversationSegment.json"
},
{
"$ref": "dateSegment.json"
},
{
"$ref": "emailClientSegment.json"
},....
distinct call will not help in this case, it just decreases count of avro records in union, but anyway all records will have name "conditions.items" and that is not allowed in avro union
There was a problem hiding this comment.
I am not sure what the conditions.items is. It's not in any of the Mailchimp modified in this PR.
Following the above example for Mailchimp, what would be the final schema after running it through the converter?
There was a problem hiding this comment.
@tuliren reverted changes from JsonToAvroSchemaConverter. Please advice if anything else should be implemented in scope of this task
There was a problem hiding this comment.
Destination S3 can parse $ref in schema if reference leads to a schema that is already in the repository.
Oh, I did not realize this. Actually where is the code that is handing $ref when the schema is in the repo?
Can you add some test cases in type_conversion_test_cases.json andjson_conversion_test_cases.json?
@tuliren sorry for the misunderstanding. Destination by itself cannot parse fields with $ref, I meant that json schema is transmitted without $ref when sync, if such fields are found in the source Example: Source Facebook Marketing (ads_insights stream) ---> Destination S3 |
|
/test connector=connectors/source-mailchimp
|
github-actions
bot
added
the
area/documentation
VitaliiMaltsev
changed the title
|
/publish connector=connectors/source-mailchimp
|
* create mailchimp schemas * remove star import * refactoring * resolve merge conflicts * update mailchimp cdk version * revert changes from JsonToAvroSchemaConverter * revert changes from JsonToAvroSchemaConverter * bump Source Mailchimp Co-authored-by: vmaltsev <[email protected]>
What
According to Slack thread destination can not parse JSON schema with $ref
How
Destination S3 can parse $ref in schema if reference leads to a schema that is already in the repository(for source connector).
Updated and added Mailchimp schemas
Recommended reading order
x.javay.pythonPre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/SUMMARY.mddocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampledocs/integrations/README.mdairbyte-integrations/builds.mdAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing./publishcommand described hereUpdating a connector
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing./publishcommand described hereConnector Generator
-scaffoldin their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplatesthen checking in your changes