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

CosmosDB - Locations field missing from model #6823

Closed
stefangordon opened this issue Aug 17, 2019 · 10 comments · Fixed by Azure/azure-rest-api-specs#7336
Closed

CosmosDB - Locations field missing from model #6823

stefangordon opened this issue Aug 17, 2019 · 10 comments · Fixed by Azure/azure-rest-api-specs#7336
Assignees
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization. Mgmt This issue is related to a management-plane library. Service Attention Workflow: This issue is responsible by Azure service team.
Milestone

Comments

@stefangordon
Copy link

stefangordon commented Aug 17, 2019

It seems the properties.locations field is not in the model
https://github.com/Azure/azure-sdk-for-python/blob/master/sdk/cosmos/azure-mgmt-cosmosdb/azure/mgmt/cosmosdb/models/database_account.py

However it is required to update the account:
https://docs.microsoft.com/en-us/rest/api/cosmos-db-resource-provider/databaseaccounts/createorupdate#request-body

The field comes across in the HTTP response from a GET just fine, it is just lost in deserialization.

This seem to make it hard for me to update a CosmosDB account (e.g. change a firewall) as the API does not appear to fully support PATCH (only for tags?) and I can't get the data I need to do acreate or update.

What am I missing here?

@shurd as you handled a similar issue :)

@johanste
Copy link
Member

The CreateOrUpdate method actually takes a DatabaseAccountCreateUpdateParameters instance.

@stefangordon
Copy link
Author

stefangordon commented Aug 19, 2019

Yeah so that model requires locations which is missing from your swagger for GET (and thus the database_account model), so I can't fill out locations to successfully make an update call.

Prior to doing an update I'd need the full copy of the resource from a GET to safely update a field, so the database_account model needs to work.

Ideally I'd PATCH so I did not have to provide a locations, but the PATCH API is also not implemented correctly and only supports tags.

It looks like the Azure CLI just manually hacks the readLocations into locations since the invalid model loses the locations during deserialization.

Seems like in current situation you can't really programmatically update the account without some trickery that may or may not be safe.

I believe that fixing the swagger and generating the models again so that the full HTTP response from GET / LIST is available in the model would allow users access to the data they need to make updates.

@stefangordon
Copy link
Author

This appears to be the incorrect swagger file responsible for bad models when retrieving current state
https://github.com/Azure/azure-rest-api-specs/blob/master/specification/cosmos-db/resource-manager/Microsoft.DocumentDB/stable/2015-04-08/cosmos-db.json#L4272

Locations comes back in the HTTP response, but isn't accessible via SDK.

@stefangordon
Copy link
Author

This is the best I've come up with to try to update an account

(I'm using the raw dictionary here instead of the model, but the same applies either way)

I'm concerned this isn't safe? Could readLocations be different than locations?

        resource = *GET THE ACCOUNT RESOURCE FROM CLIENT AS_DICT*

        resource['properties']['locations'] = []

        for loc in resource['properties'].get('readLocations'):
            resource['properties']['locations'].append(
                {'location_name': loc['locationName'],
                 'failover_priority': loc['failoverPriority'],
                 'is_zone_redundant': loc.get('isZoneRedundant', False)})

        # Update resource
        self.client.database_accounts.create_or_update(
            resource['resourceGroup'],
            resource['name'],
            create_update_parameters=resource
        )

@loarabia loarabia added Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization. Mgmt This issue is related to a management-plane library. Service Attention Workflow: This issue is responsible by Azure service team. labels Aug 19, 2019
@ghost
Copy link

ghost commented Aug 19, 2019

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @shurd

1 similar comment
@ghost
Copy link

ghost commented Aug 19, 2019

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @shurd

@shurd
Copy link

shurd commented Aug 20, 2019

Hi @stefangordon thank you for raising this issue. Every region on an account is a readable location, so you can use these from the GET response to form an update request. The code snippet you provided looks to be correct.

Right now, our PATCH request only supports updating tags as this behavior is required by ARM. We are currently working on supporting PATCH for all properties on the account.

@johanste
Copy link
Member

And if Locations actually is returned, then the swagger should probably be updated to reflect this fact.

@shurd shurd self-assigned this Sep 4, 2019
@shurd shurd added the bug This issue requires a change to an existing behavior in the product in order to be resolved. label Sep 16, 2019
@shurd
Copy link

shurd commented Sep 16, 2019

We will include locations in the next swagger update, which is expected in the next two weeks. We are tracking this internally as work item 509483.

@ghost
Copy link

ghost commented Oct 30, 2019

Thanks for working with Microsoft on GitHub! Tell us how you feel about your experience using the reactions on this comment.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization. Mgmt This issue is related to a management-plane library. Service Attention Workflow: This issue is responsible by Azure service team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants