search

theforeman / foreman_azure_rm

Adds Azure Resource Manager as a compute resource for The Foreman
GNU General Public License v3.0
9 stars 24 forks source link

don't include sub_id in api parameters #122

Closed evgeni closed 3 years ago

evgeni commented 3 years ago

we alias this to user in models/foreman_azure_rm/azure_rm.rb:

alias_attribute :sub_id, :user

and it feels more natural to use "user" instead of the subscription id

this will most probably require changes to hammer and docs, so please review carefully :)

chris1984 commented 3 years ago

Hammer PR https://github.com/theforeman/hammer_cli_foreman_azure_rm/pull/16

evgeni commented 3 years ago

Well, my idea was that you can use the --user flag for the subscription id (as that's what it would end stored as anyways).

The main question is: is this user-friendly, or shall we keep the "sub_id" a "special case" for Azure, as that's how Azure works?

evgeni commented 3 years ago

So I mentioned on IRC I might take this PR back, so here is my reasoning:

Initially, I thought that it's weird to expose the "Subscription ID" as it's internally saved as user. However, after thinking how Azure and Foreman UI always calls the data "subscription id", I think it would be actually bad for the user if we make the API call this field user (especially as there is no concept of "user" in the Azure API).

Merging this PR would require changes to hammer, docs, and still confuse users.

Thus I am closing the PR, and would ask you to do the same with your hammer part of it.