-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[Synapse] Add pipeline, linked service, trigger, notebook, data flow and dataset related cmdlets #15296
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
Conversation
a006fef to
cfa1106
Compare
|
Synapse |
|
@haroldrandom could you please help merge the PR? |
| with self.command_group('synapse linked-service', synapse_linked_service_sdk, | ||
| custom_command_type=get_custom_sdk('artifacts', None)) as g: | ||
| g.custom_command('create', 'create_or_update_linked_service', supports_no_wait=True) | ||
| g.custom_command('set', 'create_or_update_linked_service', supports_no_wait=True) |
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.
set [](start = 26, length = 3)
Usually we use "update" which is a patch like operation. Rarely to use "set". Please refer to
https://github.com/Azure/azure-cli/blob/dev/doc/command_guidelines.md#standard-command-types for more details.
please also update other "set" commands.
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.
The things these commands do are more like "set", will replace all properties of a resource without preserving existing values. Because there have the cmdlets Set-AzSynapsexxx in powershell which alias is New-AzSynapsexxx. So I use "set" here. Maybe I can delete the "set" command directly, because it has the same function with "create". What do you think about?
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.
| # Data Plane Commands --Artifacts pipeline run operations | ||
| with self.command_group('synapse pipeline-run', synapse_pipeline_run_sdk, | ||
| custom_command_type=get_custom_sdk('artifacts', None)) as g: | ||
| g.custom_command('query-by-workspace', 'query_pipeline_runs_by_workspace') |
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.
For simplicity, using "query" as command is enough?
Please also take care of other "query-xxx-xxx" commands.
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.
It keep align with ADF. In ADF, it is "datafactory pipeline-run query-by-factory".
| client = cf_synapse_trigger(cmd.cli_ctx, workspace_name) | ||
| properties = Trigger.from_dict(definition_file['properties']) | ||
| return sdk_no_wait(no_wait, client.begin_create_or_update_trigger, | ||
| trigger_name, properties, polling=True) |
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.
Remove "polling=True" as sdk_no_wait() has such logic.
Please update other places.
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.
If remove "polling=True", it can't poll the result. Just return the result with "state": "Creating". Maybe it causes by track 2 sdk?
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.
If users don't provide --no-wait, the default value of polling is True. No need to add it here. @
In reply to: 506169871 [](ancestors = 506169871)
| g.custom_command('list', 'list_triggers') | ||
| g.custom_show_command('show', 'get_trigger') | ||
| g.custom_command('delete', 'delete_trigger', confirmation=True, supports_no_wait=True) | ||
| g.custom_command('subscribe-to-event', 'subscribe_trigger_to_events', supports_no_wait=True) |
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.
For simplicity, using "subscribe" as command is enough? also for "unsubscribe".
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.
It keeps align with ADF too.
|
@wonner, I added some comments, please take a look. |
jsntcy
left a comment
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.
![]()
Description
Add some new commands to manage pipeline, linked service, trigger, notebook, data flow and dataset in synapse workspace.
az synapse linked-servicecreate/set/list/show/deleteaz synapse datasetcreate/set/list/show/deleteaz synapse pipelinecreate/set/list/show/delete/create-runaz synapse pipeline-runshow/cancel/query-by-workspaceaz synapse activity-runquery-by-pipeline-runaz synapse triggercreate/set/list/show/delete/subscribe-to-event/get-event-subscription-status/unsubscribe-from-event/start/stopaz synapse trigger-runquery-by-workspace/rerunaz synapse data-flowcreate/set/list/show/deleteaz synapse notebookcreate/set/list/show/delete/import/exportTesting Guide
az synapse linked-service -h
az synapse dataset -h
az synapse pipeline -h
az synapse pipeline-run -h
az synapse activity-run -h
az synapse trigger -h
az synapse trigger-run -h
az synapse data-flow -h
az synapse notebook -h
History Notes
This checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.