weizhouapache commented on pull request #4200: URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-902568511
> Thanks for fixing the conflicts @ravening. > > ## Here are my two cents: > From what I understood, the main discussion has been due to confusion around the scope and goals of this PR; I would like to bring some consensus and align the goals and expectations so we all _speak the same language_. > > **Note:** this feedback is based on what I understood out of a quick check on all comments. Please let me know if I am missing some points here. > > ### Main conserns > If I understood it correctly, the main issue raised for this PR was regarding the fact that Domain admins **cannot** create public offerings. > > I totally understand and agree with @PaulAngus' concerns; we need to keep the behavior of **Domain admins** creating only **non-public** offerings. If that was not the case then we would need to escalate to a deeper discussion to decide whether to change such behavior on the API or not. However, I think that this PR properly avoids the creation of public offerings by domain admins. > > ### Conclusion > I am still **+1** on having this merged, I don't see any harm in domain admins setting offerings to its domain, by default. When a ROOT admin creates an offering it is already set on its own domain. The code and automated/manual tests are aligned with the description. > > The only point that may need some attention is regarding the documentation. According to the [API documentation](https://cloudstack.apache.org/api/apidocs-4.15/apis/createServiceOffering.html) a NULL _domainid_ results in a public offering, which is not the case of domain admins; on the other side, it is not a required parameter. > > > domainid: the ID of the containing domain(s), null for public offerings` > > #### Pros: > 1. personally I think it enhances the user experience, adding flexibility and avoiding "unnecessary" error messages > 2. I myself expected _domain id_ to not be required due to the API description > 3. it keeps the offerings **NOT** public for domain-admins, regardless of the _domainid_ > 4. offerings created by the domain admins are still under his/her own domain > 5. domain admins can still create offerings for sub-domains by adding the sub-domain ID > > #### Cons: > 1. We might need to update the API description by stating the behavior when domain-admins keep it NULL @ravening could you update api descrition as suggested by @GabrielBrascher ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
