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.

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

[T2] appconfiguration #8853

Closed
wants to merge 1 commit into from
Closed

Conversation

00Kai0
Copy link
Contributor

@00Kai0 00Kai0 commented Mar 27, 2020

When I try to test appconfiguration, I got two question:

  1. miss privateEndpointConnections in the response of configuration store. Then response example:
{
  "type":"Microsoft.AppConfiguration/configurationStores",
  "properties":{
    "provisioningState":"Succeeded",
    "creationDate":"2020-03-25T04:21:41+00:00",
    "endpoint":"https://configuration39c8158a.azconfig.io",
    "encryption":{"keyVaultProperties":null},
    "privateEndpointConnections":[
      {
        "id":"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test_cli_mgmt_appconfiguration_test_appconfiguration39c8158a/providers/Microsoft.AppConfiguration/configurationStores/configuration39c8158a/privateEndpointConnections/endpointxyz.f54b944c-7053-40e1-9791-5b8297d35d18",
        "name":"endpointxyz.f54b944c-7053-40e1-9791-5b8297d35d18",
        "type":"Microsoft.AppConfiguration/configurationStores/privateEndpointConnections",
        "properties":{
          "provisioningState":"Succeeded",
          "privateEndpoint":{
            "id":"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test_cli_mgmt_appconfiguration_test_appconfiguration39c8158a/providers/Microsoft.Network/privateEndpoints/endpointxyz"
          },
          "privateLinkServiceConnectionState":{
            "status":"Approved",
            "description":"Auto-Approved",
            "actionsRequired":"None"
          }
        }
      }
    ]
  },
  "sku":{"name":"standard"},
  "id":"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test_cli_mgmt_appconfiguration_test_appconfiguration39c8158a/providers/Microsoft.AppConfiguration/configurationStores/configuration39c8158a",
  "name":"configuration39c8158a",
  "location":"westus",
  "tags":{"my_tag":"myTagValue"}
}
  1. when I try to create or update private endpoint connection, I always get an error:
azure.core.exceptions.HttpResponseError: (InvalidProperty) Some of the properties of 'PrivateEndpointConnection' are invalid. Errors: 'Missing required property 'Id'.'

So i think this id can not be ignore in request.
And also I want to find out whether appconfiguration can create private endpoint by itself.
In other services, private endpoint is created by network

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 27, 2020

azure-sdk-for-go

⚠️ warning [Logs] [Expand Details]

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 27, 2020

azure-sdk-for-java

️✔️ succeeded [Logs] [Expand Details]

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 27, 2020

azure-cli-extensions

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 Mar 27, 2020

azure-sdk-for-js

️✔️ succeeded [Logs] [Expand Details]
  • ️✔️ Generate from 0714ef5 with merge commit 663972df205d81c37d7ce5d7481bdd699af504c9. SDK Automation 13.0.17.20200326.3
  • ️✔️@azure/arm-appconfiguration [Logs]  [Preview SDK Changes]
    [npmPack] npm WARN lifecycle @azure/arm-appconfiguration@1.1.0~prepack: cannot run in wd @azure/arm-appconfiguration@1.1.0 npm install && npm run build (wd=/z/work/azure-sdk-for-js/sdk/appconfiguration/arm-appconfiguration)

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 27, 2020

azure-sdk-for-net

️✔️ succeeded [Logs] [Expand Details]
  • ️✔️ Generate from 0714ef5 with merge commit 663972df205d81c37d7ce5d7481bdd699af504c9. SDK Automation 13.0.17.20200326.3
  • ️✔️Microsoft.Azure.Management.AppConfiguration [Logs]  [Preview SDK Changes]
      No Artifact Generated.

    @openapi-sdkautomation
    Copy link

    openapi-sdkautomation bot commented Mar 27, 2020

    azure-sdk-for-python

    - Breaking Change detected in SDK

    ⚠️ warning [Logs] [Expand Details]
    • ⚠️ Generate from 0714ef5 with merge commit 663972df205d81c37d7ce5d7481bdd699af504c9. SDK Automation 13.0.17.20200326.3
    • ⚠️azure-mgmt-appconfiguration [Logs]  [Preview SDK Changes] Breaking Change Detected
      [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] /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
      [build_package]   warnings.warn(msg)
      [breaking_change_setup] Ignoring mock: markers 'python_version <= "2.7"' don't match your environment
      [breaking_change_setup] You must give at least one requirement to install (see "pip help install")
      [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 3.361% size of original (original: 16426 chars, delta: 552 chars)
      [ChangeLog] **Features**
      [ChangeLog] 
      [ChangeLog]   - Model ConfigurationStore has a new parameter private_endpoint_connections
      [ChangeLog] 
      [ChangeLog] **Breaking changes**
      [ChangeLog] 
      [ChangeLog]   - Operation PrivateEndpointConnectionsOperations.create_or_update has a new signature
      [ChangeLog]   - Operation PrivateEndpointConnectionsOperations.create_or_update has a new signature

    @@ -1374,8 +1382,7 @@
    "properties": {
    "id": {
    "description": "The resource ID.",
    "type": "string",
    "readOnly": true
    Copy link
    Member

    Choose a reason for hiding this comment

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

    It's breaking change for removing readonly.

    @raych1 raych1 added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Mar 27, 2020
    @majastrz majastrz added ARMReviewInProgress and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Mar 30, 2020
    @majastrz
    Copy link
    Member

    Please address Ray's breaking change comment. Otherwise, changes look good from ARM side.

    @00Kai0
    Copy link
    Contributor Author

    00Kai0 commented Mar 31, 2020

    Don't merge

    @raych1 raych1 added the DoNotMerge <valid label in PR review process> use to hold merge after approval label Apr 8, 2020
    @raych1 raych1 added the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Apr 18, 2020
    @zikalino zikalino changed the title fix error in appconfiguration [T2 ]appconfiguration Apr 29, 2020
    @zikalino zikalino changed the title [T2 ]appconfiguration [T2] appconfiguration Apr 29, 2020
    @raych1
    Copy link
    Member

    raych1 commented May 12, 2020

    @00Kai0 , can you please resolve the branch conflicts and the comments?

    @00Kai0
    Copy link
    Contributor Author

    00Kai0 commented May 15, 2020

    @raych1 sorry, too late. This issue seems to have been fixed by another one. #9002
    it could be closed, thx.

    @raych1 raych1 closed this May 15, 2020
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review DoNotMerge <valid label in PR review process> use to hold merge after approval
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    4 participants