opened
nitzmahone
username_0
The current implementation of profile-versioned operations properties on the main client class will override the requested REST API version in favor of the one defined in the internal default profile.

eg:
```
c = ComputeManagementClient(adalcred, subscription_id=subid, api_version='2016-04-30-preview', base_url=active_cloud.endpoints.resource_manager)

print(c.virtual_machines.api_version)
print(c.disks.api_version)
```

I'd expect to get `2016-04-30-preview` for both of these, but since `disks` is defined in the internal profile, it uses that value first (returning `2017-03-30` in this case). Short of supplying a different profile, there doesn't appear to be a way to get the requested API version for versioned operations sets defined by the default profile.

This can be worked around by passing an empty dictionary for the profile, but it's not intuitive without looking at the SDK source to see the default profile behavior, and I'm not sure if it's "kosher" (eg, is it safe to assume that the profile will always be a simple dictionary, and that empty is OK?)...
api_version is ignored for sub-versioned operations with SdkProfile
issue
created
lmazuel
username_1
@username_0 That's the behavior I wanted: api_version as a default value less important than profile. For instance, if you write:
```python
print(c.resource_skus.api_version)
```
'2016-04-30-preview' does not exists for this one, so I have to use the profile, so the idea was:
- Use profile first
- If no key in profile, fallback to api_version
- If still nothing, fail

This indeed means that if you don't change the profile, changing api_version is a partial change. It's why I updated the doc:
```python
    :param str api_version: API version to use if no profile is provided, or if
     missing in profile.
```

If you didn't find it obvious, I'm a little concerned, I expect SDK to be obvious so you don't ask yourself this kind of question.

Did you expect `c.resource_skus` to fail because there is no `2016-04-30-preview` for this one?
comment
created
nitzmahone
username_0
I think in the grand scheme of things, once profiles are "first class citizens", this is probably the behavior we want... Probably more to the point: nobody will want/care about specific api_versions in the face of properly-formed and fully-supported profiles, because that's the only way end-users will interface with it.

The question you ask is another one that I went back to the source and did lots of Googling around for the underlying REST API's versioning behavior to try and figure out, as I (wrongly) assumed the entire provider as-currently-defined was released under a given version, not that topic areas could be versioned separately. I went back and found a comment you made on a different Ansible issue to that effect (after the fact- it was an issue I didn't get notifications on). So now that I understand how that all works, it makes sense, but it seems like both the Azure REST API docs and the SDK docs could be clearer about that versioning behavior (your new docstring on `api_version` definitely helps).
comment
created
nitzmahone
username_0
(because I did wonder exactly "what happens if I ask for an operations class that's not defined for a given REST API version, which led me down a long Google/"to the code!" rabbithole ;) )
comment
created
lmazuel
username_1
Hi @username_0 
I thoughts about this one, and I think this is confusing and you're right. I will release soon the new major version (kwargs only args, new LRO implementation, etc.) and I want to take this opportunity to refactor this one as well.

I want to:
- Make `api_version` and `profile` mutually exclusive. This solves this issue here. Profile can take a `None` key if you want to say "everything else I didn't define explicitly".
- I'm adding a `Profile` class. By default the new signature will then be:
```python
def __init__(self, credentials, subscription_id, api_version=None, base_url=None, profile=Profile.default)
```
And `Profile.default` will be by default valued `Profile.latest`.

Meaning you can create your client this way if you want to explictly say "I want latest":
```python
client = ComputeMangementClient(creds, sub, profile=Profile.latest)
```
I will support the known profiles as well:
```python
client = ComputeMangementClient(creds, sub, profile=Profile.v2017_09_03)
```
`profile`can still get a dictionnary as right now, I don't change that.

If you use `default`, you could also change the default profile as the application level:
```python
Profile.default = Profile.v2017_09_03
```
This would mutate all created client automatically to the new profile if you do that in the middle of your application. 

I would really appreciate if you can give me your thoughts on this :)
Thanks!

FYI @johanste
comment
closed
lmazuel
username_1
issue
created
lmazuel
username_1
Everything I just said was released recently, and I even made Compute/Resource as stable 4.0.0/2.0.0.
Feel free to email me or comment if necessary :)
Thanks!
comment