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

CDK: improve TypeTransformer to convert simple types to array of simple types #16636

Merged
merged 11 commits into from
Sep 15, 2022

Conversation

grubberr
Copy link
Contributor

@grubberr grubberr commented Sep 13, 2022

Signed-off-by: Sergey Chvalyuk grubberr@gmail.com

What

I think we can improve TypeTransformer to convert simple type (string, number, integer, boolean, null) values
to array of this values if schema requires array of simple type

For example:

schema = {
    "type": "object",
    "properties": {
        "value": {"type": "array"}
    }
}

{"value": 1} -> {"value": [1]}
{"value": "string"} -> {"value": ["string"]}
{"value": "string"} -> {"value": ["string"]}
{"value": True} -> {"value": [True]}
{"value": 1.5} -> {"value": [1.5]}
  1. If value is not simple type ("array", "object") we DON'T DO such conversion:
{"value": {"key": "value"}} -> {"value": {"key": "value"}}  #  nothing changed
  1. if array-items is complex type DO nothing
schema = {
    "type": "object",
    "properties": {
        "value": {
            "type": "array",
            "items": {"type": ["object"]}   # <- complex type
        }
    }
}

{"value": 10} -> {"value": 10}  # nothing changed 
  1. additional conversions inside array still works as before
schema = {
    "type": "object",
    "properties": {
        "value": {
            "type": "array",
            "items": {"type": ["string"]}
        }
    }
}

{"value": 10} -> {"value": ["10"]}

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr grubberr requested a review from a team as a code owner September 13, 2022 13:12
@grubberr grubberr removed the request for review from a team September 13, 2022 13:12
@github-actions github-actions bot added the CDK Connector Development Kit label Sep 13, 2022
@grubberr grubberr self-assigned this Sep 13, 2022
@grubberr grubberr changed the title CDK: make TypeTransformer convert simple types to array if needed CDK: improve TypeTransformer to convert simple types to array of simple types Sep 13, 2022
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr grubberr requested a review from a team September 13, 2022 22:38
@grubberr grubberr removed the request for review from a team September 14, 2022 10:58
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

Good addition! Just for simplicity, could you release only 1 version of the CDK containing all 2-3 changes you recently made in the separate PRs?

@grubberr
Copy link
Contributor Author

@sherifnada ok I will publish together with #16695

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr
Copy link
Contributor Author

grubberr commented Sep 15, 2022

/publish-cdk dry-run=true

🕑 https://github.com/airbytehq/airbyte/actions/runs/3058759143
https://github.com/airbytehq/airbyte/actions/runs/3058759143

@grubberr
Copy link
Contributor Author

grubberr commented Sep 15, 2022

/publish-cdk dry-run=false

🕑 https://github.com/airbytehq/airbyte/actions/runs/3058864811
https://github.com/airbytehq/airbyte/actions/runs/3058864811

@grubberr grubberr merged commit 74e936b into master Sep 15, 2022
@grubberr grubberr deleted the grubberr/15405-airbyte_cdk_transform_fix_3 branch September 15, 2022 08:34
letiescanciano added a commit that referenced this pull request Sep 15, 2022
* master: (200 commits)
  🪟 🧹 Display returned error messages on replication view (#16280)
  🎉 Source mixpanel: Use "Retry-After" header for backoff (#16770)
  🐛 Source google ads: mark custom query fields required (#15858)
  🪟 🔧Remove useRouter hook (#16598)
  CDK: improve TypeTransformer to convert simple types to array of simple types (#16636)
  CDK: TypeTransformer - warning message more informative (#16695)
  Source MySQL: Add Python SAT to detect backwards breaking changes (#16445)
  remove eager (#16756)
  bump com.networknt:json-schema-validator to latest version (#16619)
  Remove Cloud from Kafka docs (#16753)
  Normalization Summaries table and read/write methods (#16655)
  comment out flaky test suite while it is being investigated (#16752)
  Update ConfigRepository to read protocol version (#16670)
  Use LOG4J2 to wrap connectors logs to JSON format (#15668)
  Update connector catalog (#16749)
  🪟 🎨 Remove feedback modal from UI (#16548)
  Add missing env var for Kube overlays (#16747)
  Prepare for React v18 upgrade (#16694)
  🪟 🐛 Fix direct job linking to work with pagination (#16517)
  Fix formatting (#16743)
  ...
robbinhan pushed a commit to robbinhan/airbyte that referenced this pull request Sep 29, 2022
…le types (airbytehq#16636)

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
…le types (airbytehq#16636)

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CDK Connector Development Kit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants