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

[Batch Pool] REST API returns vmSize property in upper case ? #4956

Open
jcorioland opened this issue Dec 21, 2018 · 6 comments
Open

[Batch Pool] REST API returns vmSize property in upper case ? #4956

jcorioland opened this issue Dec 21, 2018 · 6 comments
Assignees
Labels
Batch Service Attention Workflow: This issue is responsible by Azure service team.

Comments

@jcorioland
Copy link

Hi,

I am currently working on the support of Azure Batch on the Azure Terraform provider. I am using the Azure SDK for Go and I've noticed that the vmSize for a Batch pool is returned in upper case.
See discussion here and here.
Looking at the specs, I don't see any mention of that.

The vmSize field is also all caps in the examples.

Any reason for that ?
Thanks

@jcorioland jcorioland changed the title [Batch Pool] REST API returns VMSize in upper case ? [Batch Pool] REST API returns vmSize property in upper case ? Dec 21, 2018
@akning-ms akning-ms added Batch Service Attention Workflow: This issue is responsible by Azure service team. labels Nov 27, 2020
@ghost
Copy link

ghost commented Nov 27, 2020

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @mksuni, @bgklein, @mscurrell, @cRui861, @paterasMSFT, @gingi, @dpwatrous.

Issue Details

Hi,

I am currently working on the support of Azure Batch on the Azure Terraform provider. I am using the Azure SDK for Go and I've noticed that the vmSize for a Batch pool is returned in upper case.
See discussion here and here.
Looking at the specs, I don't see any mention of that.

The vmSize field is also all caps in the examples.

Any reason for that ?
Thanks

Author: jcorioland
Assignees: jianghaolu
Labels:

Batch, Service Attention

Milestone: -

@dpwatrous
Copy link
Member

Closing since this issue is 2 years old and there's a work around in place. However, I agree that the casing is potentially confusing and we should consider changing it to match other services in a future API version.

@tombuildsstuff
Copy link
Contributor

@dpwatrous since this isn't resolved, can we re-open this and leave this issue open until this issue is resolved? Thanks!

@dpwatrous
Copy link
Member

Sure thing, I'd just like to get a better idea of what a resolved status would look like.

While I absolutely sympathize with the issue here, there are no current plans to change this behavior on the service side. Changing the canonical casing of VM sizes in Batch would be a breaking change (in fact, it would break the PR linked here), and we'd have to weigh the risk of that change against the benefits of having casing consistent with other Azure APIs.

That said, one thing we can do in the near term is to add some clarifying documentation that the GET/LIST calls will return values with casing that may vary from published VM sizes and that comparisons against them should be case insensitive. Would that help?

@dpwatrous dpwatrous reopened this Dec 1, 2020
@tombuildsstuff
Copy link
Contributor

@dpwatrous as you've mentioned I think there's two things here:

  1. documenting the behaviour of the current API version - changing the case would be a breaking change, so I'd agree this wants some documentation to reflect that, and that this may change (e.g. this field is returned in a case-insensitive manner, but may change in a future api version)
  2. fixing this in a future API version - which is why I'd like to see this left open to ensure it's fixed in time

For me "resolved" would be a fix for this shipping in the next major release version of the API, whenever that is; I get that won't be overnight and that's fine - but the reason I requested this stayed open was to ensure it wasn't "out of sight, out of mind", even if this is a secondary issue tracker - since this kind of request is easy to fall through the cracks otherwise :)

@dpwatrous
Copy link
Member

@tombuildsstuff That makes sense to me. We'll plan to improve the docs in the short term, and I'll follow up with the service team on the more thorny issue of changing our canonical casing of VM sizes. I'm fine keeping this issue open until there's a consensus on what to do there. Really appreciate the feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Batch Service Attention Workflow: This issue is responsible by Azure service team.
Projects
None yet
Development

No branches or pull requests

5 participants