Skip to content

[Synapse Management] Update sqlPool.json and add integrationRuntime.json #8628

Merged
yungezz merged 2 commits into
Azure:masterfrom
aim-for-better:UpdateSqlPoolAndIntegrationRuntime
Mar 27, 2020
Merged

[Synapse Management] Update sqlPool.json and add integrationRuntime.json #8628
yungezz merged 2 commits into
Azure:masterfrom
aim-for-better:UpdateSqlPoolAndIntegrationRuntime

Conversation

@aim-for-better
Copy link
Copy Markdown
Member

@aim-for-better aim-for-better commented Mar 8, 2020

  1. update sqlPool.json to add 202 response
  2. add integrationRuntime.json and related example

ARM team has reviewed integrationRuntime.json and related example in https://github.com/Azure/azure-rest-api-specs-pr/pull/1024
Hope this information can reduce ARM review time.

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

  • I have reviewed the documentation for the workflow.
  • Validation tools were run on swagger spec(s) and have all been fixed in this PR.
  • The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

  • Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
  • adding/removing APIs.
  • adding/removing properties.
  • adding/removing API-version.
  • adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

  • If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
    Please follow the link to find more details on API review process.

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

6 similar comments
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@azuresdkci
Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@openapi-sdkautomation
Copy link
Copy Markdown

openapi-sdkautomation Bot commented Mar 8, 2020

azure-sdk-for-go - Release

️✔️ succeeded [Logs] [Expand Details]

@openapi-sdkautomation
Copy link
Copy Markdown

openapi-sdkautomation Bot commented Mar 8, 2020

azure-sdk-for-java - Release

failed [Logs] [Expand Details]
  • Generate from 0575ac3 with merge commit 39df77d. SDK Automation 13.0.17.20200326.3
    [AutoRest] FATAL: System.Exception: Duplicate File Generation: src/main/java/com/microsoft/azure/management/synapse/v2019_06_01_preview/IntegrationRuntimeNodeIpAddress.java
    [AutoRest]    at AutoRest.Core.CodeGenerator.d__13.MoveNext() in /home/vsts/work/1/s/autorest.common/src/CodeGenerator.cs:line 151
    [AutoRest] --- End of stack trace from previous location where exception was thrown ---
    [AutoRest]    at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    [AutoRest]    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
    [AutoRest]    at AutoRest.Core.CodeGenerator.d__12.MoveNext() in /home/vsts/work/1/s/autorest.common/src/CodeGenerator.cs:line 121
    [AutoRest] --- End of stack trace from previous location where exception was thrown ---
    [AutoRest]    at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    [AutoRest]    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
    [AutoRest]    at AutoRest.Java.Azure.Fluent.CodeGeneratorJvaf.d__6.MoveNext() in /home/vsts/work/1/s/src/azurefluent/CodeGeneratorJvaf.cs:line 106
    [AutoRest] --- End of stack trace from previous location where exception was thrown ---
    [AutoRest]    at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    [AutoRest]    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
    [AutoRest]    at AutoRest.Java.Program.d__3.MoveNext() in /home/vsts/work/1/s/src/Program.cs:line 114
    [AutoRest] --- End of stack trace from previous location where exception was thrown ---
    [AutoRest]    at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    [AutoRest]    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
    [AutoRest]    at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
    [AutoRest]    at NewPlugin.d__20.MoveNext() in /home/vsts/work/1/s/autorest.common/src/Plugins/NewPlugin.cs:line 163
    [AutoRest] FATAL: java/generate - FAILED
    [AutoRest] FATAL: Error: Plugin java reported failure.
    [AutoRest] Failure during batch task - {"tag":"package-2019-06-01-preview"} -- Error: Plugin java reported failure..
    [AutoRest]   Error: Plugin java reported failure.
    Failed to run autorest.
    Error: /z/node_modules/.bin/autorest --version=2.0.4413 --java --verbose --multiapi --use=@microsoft.azure/autorest.java@2.1.95 --azure-libraries-for-java-folder=/z/work/azure-sdk-for-java /z/work/azure-rest-api-specs/specification/synapse/resource-manager/readme.md FATAL: System.Exception: Duplicate File Generation: src/main/java/com/microsoft/azure/management/synapse/v2019_06_01_preview/IntegrationRuntimeNodeIpAddress.java
       at AutoRest.Core.CodeGenerator.d__13.MoveNext() in /home/vsts/work/1/s/autorest.common/src/CodeGenerator.cs:line 151
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at AutoRest.Core.CodeGenerator.d__12.MoveNext() in /home/vsts/work/1/s/autorest.common/src/CodeGenerator.cs:line 121
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at AutoRest.Java.Azure.Fluent.CodeGeneratorJvaf.d__6.MoveNext() in /home/vsts/work/1/s/src/azurefluent/CodeGeneratorJvaf.cs:line 106
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at AutoRest.Java.Program.d__3.MoveNext() in /home/vsts/work/1/s/src/Program.cs:line 114
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
       at NewPlugin.d__20.MoveNext() in /home/vsts/work/1/s/autorest.common/src/Plugins/NewPlugin.cs:line 163
    FATAL: java/generate - FAILED
    FATAL: Error: Plugin java reported failure.
    Failure during batch task - {"tag":"package-2019-06-01-preview"} -- Error: Plugin java reported failure..
      Error: Plugin java reported failure.
    , {} 

@openapi-sdkautomation
Copy link
Copy Markdown

openapi-sdkautomation Bot commented Mar 8, 2020

azure-sdk-for-net - Release

️✔️ succeeded [Logs] [Expand Details]

@openapi-sdkautomation
Copy link
Copy Markdown

openapi-sdkautomation Bot commented Mar 8, 2020

azure-sdk-for-js - Release

️✔️ succeeded [Logs] [Expand Details]
  • ️✔️ Generate from 0575ac3 with merge commit 39df77d. SDK Automation 13.0.17.20200326.3
  • ️✔️@azure/arm-synapse [Logs]  [Release SDK Changes]
    [npmPack] npm WARN lifecycle @azure/arm-synapse@1.0.0~prepack: cannot run in wd @azure/arm-synapse@1.0.0 npm install && npm run build (wd=/z/work/azure-sdk-for-js/sdk/synapse/arm-synapse)

@openapi-sdkautomation
Copy link
Copy Markdown

openapi-sdkautomation Bot commented Mar 8, 2020

azure-sdk-for-python - Release

️✔️ succeeded [Logs] [Expand Details]
  • ️✔️ Generate from 0575ac3 with merge commit 39df77d. SDK Automation 13.0.17.20200326.3
  • ️✔️azure-mgmt-synapse [Logs]  [Release SDK Changes]
    [build_package] /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
    [build_package]   warnings.warn(msg)
    [build_package] warning: no files found matching '*.py' under directory 'tests'
    [build_package] warning: no files found matching '*.yaml' under directory 'tests'
    [build_package] /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
    [build_package]   warnings.warn(msg)
    [build_package] warning: no files found matching '*.py' under directory 'tests'
    [build_package] warning: no files found matching '*.yaml' under directory 'tests'
    [breaking_change_setup] Ignoring mock: markers 'python_version <= "2.7"' don't match your environment
    [breaking_change_setup] Cannot uninstall requirement azure-nspkg, not installed
    [breaking_change_setup] Command '['/usr/local/bin/python', '-m', 'pip', 'uninstall', '-y', 'azure-nspkg']' returned non-zero exit status 1.
    [ChangeLog] Size of delta 36.832% size of original (original: 110838 chars, delta: 40824 chars)
    [ChangeLog] **Features**
    [ChangeLog] 
    [ChangeLog]   - Added operation group IntegrationRuntimeAuthKeysOperations
    [ChangeLog]   - Added operation group IntegrationRuntimeNodeIpAddressOperations
    [ChangeLog]   - Added operation group IntegrationRuntimeCredentialsOperations
    [ChangeLog]   - Added operation group IntegrationRuntimeMonitoringDataOperations
    [ChangeLog]   - Added operation group IntegrationRuntimesOperations
    [ChangeLog]   - Added operation group IntegrationRuntimeObjectMetadataOperations
    [ChangeLog]   - Added operation group IntegrationRuntimeConnectionInfosOperations
    [ChangeLog]   - Added operation group IntegrationRuntimeStatusOperations
    [ChangeLog]   - Added operation group IntegrationRuntimeNodesOperations

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@aim-for-better aim-for-better force-pushed the UpdateSqlPoolAndIntegrationRuntime branch from 7bd348f to 0dd8662 Compare March 9, 2020 05:25
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@aim-for-better aim-for-better force-pushed the UpdateSqlPoolAndIntegrationRuntime branch from 0dd8662 to afd6796 Compare March 10, 2020 10:45
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Member

@yungezz yungezz Mar 10, 2020

Choose a reason for hiding this comment

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

just curious, why use post get here? #Resolved

Copy link
Copy Markdown
Member

@anthony-c-martin anthony-c-martin Mar 20, 2020

Choose a reason for hiding this comment

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

If that is the case, then why are the body parameters not modeled in this swagger file?

Hi @anthony-c-martin , I am sorry for giving a wrong reason for why it it post.
I checked with backend team, they said it is a action instead of simply get resource, the action need to be audited and loged. So it is Post instead of Get. Thanks

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same as above

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

same as above

Checked with backend, this api requires parameters to be passed in the body.

@yungezz yungezz added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Mar 10, 2020
Copy link
Copy Markdown
Member

@fengzhou-msft fengzhou-msft left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you have a default value for ttl?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Do you have a default value for ttl?

No

2. add integrationRuntime.json and related example
@aim-for-better aim-for-better force-pushed the UpdateSqlPoolAndIntegrationRuntime branch from afd6796 to 45d940b Compare March 11, 2020 03:37
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@aim-for-better
Copy link
Copy Markdown
Member Author

@yungezz , Can we remove WaitForARMFeedback label? ARM team has reviewed integrationRuntime.json and related example in Azure/azure-rest-api-specs-pr#1024

@yungezz
Copy link
Copy Markdown
Member

yungezz commented Mar 11, 2020

@yungezz , Can we remove WaitForARMFeedback label? ARM team has reviewed integrationRuntime.json and related example in Azure/azure-rest-api-specs-pr#1024

hi @arm-for-better, I compared integrationRuntime.json in this PR with the one in https://github.com/Azure/azure-rest-api-specs-pr/pull/1024/, there're lots of diffs including new operations like "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Synapse/workspaces/{workspaceName}/integrationRuntimes/{integrationRuntimeName}/disableInteractiveQuery", "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Synapse/workspaces/{workspaceName}/integrationRuntimes/{integrationRuntimeName}/removeNode": {
, and change on request paramters. This need ARM review again

@aim-for-better
Copy link
Copy Markdown
Member Author

@yungezz , Can we remove WaitForARMFeedback label? ARM team has reviewed integrationRuntime.json and related example in Azure/azure-rest-api-specs-pr#1024

hi @arm-for-better, I compared integrationRuntime.json in this PR with the one in Azure/azure-rest-api-specs-pr#1024, there're lots of diffs including new operations like "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Synapse/workspaces/{workspaceName}/integrationRuntimes/{integrationRuntimeName}/disableInteractiveQuery", "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Synapse/workspaces/{workspaceName}/integrationRuntimes/{integrationRuntimeName}/removeNode": {
, and change on request paramters. This need ARM review again

Ok, It makes sense to me.

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@openapi-sdkautomation
Copy link
Copy Markdown

openapi-sdkautomation Bot commented Mar 16, 2020

azure-cli-extensions - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@PhoenixHe-NV
Copy link
Copy Markdown

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

@aim-for-better
Copy link
Copy Markdown
Member Author

@anthony-c-martin could you please help do the ARM review? we are blocked on ARM review for a long time. Thanks in advance.

Comment thread custom-words.txt Outdated
exportdevices
exporterrors
exportstatus
Exprired
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

?

Copy link
Copy Markdown
Member Author

@aim-for-better aim-for-better Mar 20, 2020

Choose a reason for hiding this comment

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

?
there is a property named "isIdentityCertExprired"
backend team said it's typo but they can't change it because it will cause breaking change. So in order to pass validation i have to add this as custom word

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This review hasn't been signed off on yet, so I don't see how it's possible to have a breaking change. Please ask the backend team to fix this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@fengzhou-msft, @yungezz is there any way to merge this PR in without adding Exprired to the custom-words list?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@fengzhou-msft, @yungezz is there any way to merge this PR in without adding Exprired to the custom-words list?

Hi @anthony-c-martin I have removed Exprired from the custom-words list and added it to cSpell.json to suppress spell check

"type": "string"
}
},
"additionalProperties": {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What additional properties are allowed here? Can they be modeled in swagger?

Copy link
Copy Markdown
Member Author

@aim-for-better aim-for-better Mar 23, 2020

Choose a reason for hiding this comment

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

What additional properties are allowed here? Can they be modeled in swagger?

checked with backend team, they said it was a work around to keep the sdk compatible because backend returns more properties as time goes.

Copy link
Copy Markdown
Member

@anthony-c-martin anthony-c-martin Mar 20, 2020

Choose a reason for hiding this comment

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

If that is the case, then why are the body parameters not modeled in this swagger file?

Hi @anthony-c-martin , I am sorry for giving a wrong reason for why it it post.
I checked with backend team, they said it is a action instead of simply get resource, the action need to be audited and loged. So it is Post instead of Get. Thanks

}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Synapse/workspaces/{workspaceName}/integrationRuntimes/{integrationRuntimeName}/nodes/{nodeName}/ipAddress": {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please use a verb here - e.g getIpAddress rather than just ipAddress

Copy link
Copy Markdown
Member Author

@aim-for-better aim-for-better Mar 20, 2020

Choose a reason for hiding this comment

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

Please use a verb here - e.g getIpAddress rather than just ipAddress

Hi anthony, change api needs backend team support. The backend just use ipAddress.
Please wait me for checking this with the backend team

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Please use a verb here - e.g getIpAddress rather than just ipAddress

Hi @anthony-c-martin I understand your point and I agree with you. But the datafactory is an existing service, synapse reused their some functionality, backend team won't change the api path because the effort is large. Can we ignore this?

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@anthony-c-martin anthony-c-martin added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Mar 26, 2020
@aim-for-better aim-for-better requested a review from yungezz March 26, 2020 16:26
@aim-for-better
Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

@yungezz yungezz added Approved-BreakingChange DO NOT USE! OBSOLETE label. See https://github.com/Azure/azure-sdk-tools/issues/6374 potential-sdk-breaking-change labels Mar 27, 2020
@yungezz yungezz merged commit 39df77d into Azure:master Mar 27, 2020
00Kai0 pushed a commit to 00Kai0/azure-rest-api-specs that referenced this pull request Oct 12, 2020
@nschonni nschonni mentioned this pull request Jan 24, 2021
25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved-BreakingChange DO NOT USE! OBSOLETE label. See https://github.com/Azure/azure-sdk-tools/issues/6374 ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review potential-sdk-breaking-change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants