-
Notifications
You must be signed in to change notification settings - Fork 4k
[Synapse] - Fixed deserialization error when create Pipeline/Dataset/Trigger through DefinitionFile #13721
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
[Synapse] - Fixed deserialization error when create Pipeline/Dataset/Trigger through DefinitionFile #13721
Conversation
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
{ | ||
internal static T DeserializeObject<T>(string rawJsonContent) | ||
{ | ||
var document = JsonDocument.Parse(rawJsonContent, default); |
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.
Feature default literal
is not available in C# 7.0.
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.
Thanks for the suggestion. Fixed.
@wyunchi-ms could you please help review again? |
Could you please add these modification into |
@wyunchi-ms there is already an entry in the Upcoming Release section:
|
Description
<TL;DR>
This PR aims to fix deserialization issue for New-*Pipeline/Dataset/Trigger cmdlets.
TL:DR
Synapse has integrated some Azure Data Factory's fundamental features, for example pipeline, dataset, triggers, etc. We refer these resources as artifacts. One common use scenario for New-*Artifact is, users provide a definition file that contains request body, the PowerShell command reads the definition file, deserialize its content to some SDK models and call methods to send a PUT request to create some artifact resource.
Previously, these related PowerShell cmdlets suffer from a deserialization issue. In short, this is because, The PowerShell consumes Azure.Analytics.Synapse.Artifacts C# SDK. The SDK is track 2 data plane SDK. Its serializer/deserializer (methods to serialize/deserialize SDK models) are not exposed. For example, see PipelineResource.Serialization.cs - methods in this file are not
public
.Before this change, our deserialization approach in PowerShell is as below: we writes some wrapper classes, let's say PSArtifact.cs. First of call, we leverage Newtonsoft.Json library for serialization to an instance of
PSArtifact
, then we write and callToSdkObject
method insidePSArtifact
to transformPSArtifact
to some SDK modelSdkArtifact
. Then call SDK method withSdkArtifact
. This approach is buggy and we have seen several customer reported deserialization failure issues recently. At code level, this is because we have to write lots ofToSdkObject
methods by hand and provide parameterless constructor for many files (Otherwise the Newtonsoft.Json will complain that it cannot find proper ctor or call some unexpected ctor).In this PR, we are trying to fix the deserialization issue by calling SDK's original serialization/de-serialization methods, rather than leverage Newtonsoft.Json library. Due to the invisibility of those SDK serialization methods, we cannot call them directly. So we turn to use reflection instead. The mainly related files are:
Besides the major change that is mentioned above, the PR also does the following items:
Azure.Analytics.Synapse.Artifacts
to the latest version and update code inSet-AzSynapseNotebook
to solve an payload validation issue due to a breaking change at backend side.Checklist
CONTRIBUTING.md
ChangeLog.md
file(s) has been updated:ChangeLog.md
file can be found atsrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
## Upcoming Release
header -- no new version header should be added