Skip to content

Conversation

@ArcturusZhang
Copy link
Member

The identity in workspace should not be readonly, there are examples in other services you can find here, here, here and more (I only listed those in the compute service).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Dec 13, 2019

azure-sdk-for-go - Release

⚠️ warning [Logs] [Expand Details]

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Dec 13, 2019

azure-sdk-for-java - 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

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Dec 13, 2019

azure-sdk-for-python - Release

️✔️ succeeded [Logs] [Expand Details]
  • ️✔️ Generate from da48a54 with merge commit 550c4c5. SDK Automation 13.0.17.20191213.4
  • ️✔️azure-mgmt-machinelearningservices [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'

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Dec 13, 2019

azure-sdk-for-net - Release

️✔️ succeeded [Logs] [Expand Details]
  • ️✔️ Generate from da48a54 with merge commit 550c4c5. SDK Automation 13.0.17.20191213.4
    [AutoRest] realpath(): Permission denied
    [AutoRest] realpath(): Permission denied
    [AutoRest] realpath(): Permission denied
    [AutoRest] realpath(): Permission denied
    [AutoRest] realpath(): Permission denied
    [AutoRest] realpath(): Permission denied
  • ️✔️Microsoft.Azure.Management.MachineLearningServices [Logs]  [Release SDK Changes]
      No Artifact Generated.

    @openapi-sdkautomation
    Copy link

    openapi-sdkautomation bot commented Dec 13, 2019

    azure-sdk-for-js - Release

    ️✔️ succeeded [Logs] [Expand Details]
    • ️✔️ Generate from da48a54 with merge commit 550c4c5. SDK Automation 13.0.17.20191213.4
    • ️✔️@azure/arm-machinelearningservices [Logs]  [Release SDK Changes]
      [npmPack] npm WARN deprecated [email protected]: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-node-resolve.
      [npmPack] loaded rollup.config.js with warnings
      [npmPack] (!) Unused external imports
      [npmPack] default imported from external module 'rollup' but never used
      [npmPack] 
      [npmPack] ./esm/azureMachineLearningWorkspaces.js → ./dist/arm-machinelearningservices.js...
      [npmPack] created ./dist/arm-machinelearningservices.js in 416ms

    @azuresdkci
    Copy link
    Contributor

    Can one of the admins verify this patch?

    @weidongxu-microsoft
    Copy link
    Member

    @ArcturusZhang

    This change should be of low risk. However IMHO the point is whether service support changing of the property, not whether it should or should not be read-only.

    Please let me know if you need @vrushg-ms or any owner of the service to sign-off this PR.

    @ArcturusZhang
    Copy link
    Member Author

    @ArcturusZhang

    This change should be of low risk. However IMHO the point is whether service support changing of the property, not whether it should or should not be read-only.

    Please let me know if you need @vrushg-ms or any owner of the service to sign-off this PR.

    I am asking the service team to have a review on this.
    Actually I already done some experiments on this rest api.
    Despite the identity is marked as readOnly, the rest api call with a body without identity will be rejected by the service saying You must call this api using a client that supports MSI which indicates that you must assign this identity property.
    If the rest api was sent with identity, everything is fine.

    I submit this pr to change the readOnly in swagger because the readOnly field will affect how the SDK is generated. The code generator will recognize this field and do not put the read only fields when the SDK is marshalling or serializing SDK-structs to the json body in the rest api. In this case, it will cause the SDK call to fail.

    @weidongxu-microsoft
    Copy link
    Member

    weidongxu-microsoft commented Dec 16, 2019

    @ArcturusZhang

    Got it.

    Another curiority is that service put Identity property into this base Resource object definition, which seems not very usual.

    @weidongxu-microsoft weidongxu-microsoft merged commit 550c4c5 into Azure:master Dec 17, 2019
    @ArcturusZhang ArcturusZhang deleted the remove-readonly-in-machinelearningservice branch December 17, 2019 01:50
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    None yet

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    4 participants