-
Notifications
You must be signed in to change notification settings - Fork 332
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
Add new module azure_rm_afdorigin
to support Azure Frontdoor Standard and Premium
#1591
base: dev
Are you sure you want to change the base?
Conversation
@jartoo Please help fix below errors:
|
resource_group: myResourceGroup | ||
state: present | ||
origin: | ||
host_name: "10.0.0.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong indentation: expected 6 but found 8
profile_name: myProfile | ||
resource_group: myResourceGroup | ||
state: absent | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
item = self.endpoint_client.afd_origins.get( | ||
resource_group_name=self.resource_group, profile_name=self.profile_name, origin_group_name=self.origin_group_name, origin_name=self.name) | ||
except Exception as exc: | ||
self.log("Did not find resource.".format(str(exc))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
too-many-format-args: Too many arguments for format string
In looking at this further, I am wondering if i should flatten the input parameters and resubmit this... Looking for architectural guidance on this. In working thru the Route module, it seems to be much cleaner when I do this and not nest the parameters under unnecessary suboptions. For example, move all of the origin suboptions, like azure_origin to the root level, reducing the nesting complexity. |
Doing this would also help with the required_if and the attribute setting sections, which do not work well with nested layers of inputs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If self.origin is None, it will not be able to get the value through the dictionary method, you should first determine 'self.origin is not None', in addition, 'self.origin 'parameters are not all necessary, Therefore, it can be obtained by 'self.origination.get('***'), and then compare it if it is not' None' after obtaining it. Thank you!
self.results['id'] = response['id'] | ||
self.results['host_name'] = response['host_name'] | ||
|
||
if response['host_name'] != self.origin['host_name'] and self.origin['host_name']: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if response['host_name'] != self.origin['host_name'] and self.origin['host_name']: | |
if self.origin is not None: | |
self.origin.get('host_name') is not None and response['host_name'] != self.origin['host_name']: |
Looking at this again after working the AFDRoute module, I think it makes sense to flatten the inputs if not in a list structure to reduce complexity and take advantage of the built-in functionality which only works at the root level, such as the required_if and required_together functions. I will rework this and submit further changes...sorry, figuring this out as I go it seems. |
@Fred-sun Please let me know if you prefer flattened inputs, per the above comment, or hierarchical ones. I have not found yet any guidance on the approach for this. It would be helpful if I had this before I rework the other pull requests. With hierarchical you lose the ability to use required_if and required_together functions as well as setting defaults as those functions currently only run on the first level in the inputs. I can rework the other module pull requests to align...Thanks! |
@jartoo Thank you very much for your reply, because we will create and delete two states。 The delete states, those are not required parameters can be None, so we need to make a judgment first. Also, regarding the new module, please add it to pr-pipeline.yml and meta/runtime.yml. Thank you! |
Let me refactor this. Put this on hold until I submit the flattened parameters...will try to get this done ASAP. |
@jartoo Ok, you can also add me to this branch of yours and I will work with you to solve the repo problem. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small change request!
azure_rm_afdorigin
to support Azure Frontdoor Standard and Premium
@jartoo can you help resolve the file conflict. |
test failed:
|
@jartoo You seem to have restored the changes of the dev branch, please check this PR again, thank you! |
SUMMARY
This module manages Frontdoor Origins for the Standard and Premium services. I propose building this as a separate module in lieu of complicating the azure_cdn_profile module so as to make the management of each of the elements easier. Azure Frontdoor is quite complex, so matching the modules with the Python SDK APIs seems to be the prudent design choice.
Continues to work on #1041 This does not complete this issue, yet.
ISSUE TYPE
COMPONENT NAME
Module: azure_rm_afdorigin
ADDITIONAL INFORMATION
Provides the ability to manage Origins, per the Python SDK here: https://learn.microsoft.com/en-us/python/api/azure-mgmt-cdn/azure.mgmt.cdn.operations.afdoriginsoperations?view=azure-python