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.

Sign up for GitHub

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 table option to copy sink #6819

Merged
merged 1 commit into from
Aug 14, 2019
Merged

Add table option to copy sink #6819

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...actory/resource-manager/Microsoft.DataFactory/stable/2018-06-01/entityTypes/Pipeline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2591,6 +2591,10 @@
"maxConcurrentConnections": {
"type": "object",
"description": "The maximum concurrent connection count for the sink data store. Type: integer (or Expression with resultType integer)."
},
"tableOption": {
Copy link
Contributor

Choose a reason for hiding this comment

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

New writable properties should go in a new api-version. what is the impact of this value being set via portal, resource retrieved via an existing SDK which ignores this value since it doesn't understand it. Now the user sends a PUT on the existing resource from the SDK but the request body doesn't contain this. what happens to the value of this property on the server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • What is the impact of this value being set via portal：
    The table in the target side will auto create for copy job.

  • What happens to the value of this property on the server if request body doesn't contain this:
    the default value is none.

This is a very common qestion for new release feature and backword compatiable
@hvermis help to explain the ADF SDK behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

The SDK will ignore it if it doesn't know about it. We keep updating our SDKs frequently to keep and ask our customers to get the latest version. We also align new features in our UI with our SDK releases, and our customers know that if they use both, they need to get the latest SDK.
This actually a downside of Autorest - it should have kept original properties instead of discarding them, but that is a different topic.

Copy link
Contributor

Choose a reason for hiding this comment

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

@hvermis - you do realize that this is contrary to the Microsoft Rest API guidelines which are publicly available? Please check it here - https://github.com/microsoft/api-guidelines/blob/vNext/Guidelines.md#123-definition-of-a-breaking-change
paragraph 2 sentence 2 in the above section.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ravbhatnagar Yes, we are aware. However those guidelines are not scalable - we always need to add new properties to our APIs and models, and adding new API version every time is not feasible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ravbhatnagar Do you have any other question? Can you help to approve this PR?

"type": "object",
"description": "The option to handle sink table, such as autoCreate. For now only 'autoCreate' value is supported. Type: string (or Expression with resultType string)."
}
},
"additionalProperties": {
Expand Down