Make OCI's availability domain required (SC-59)#147
Conversation
availability_domain used to be able to be inferred from the subnet. Now it is required to be passed manually. https://docs.oracle.com/en-us/iaas/tools/python/2.38.3/api/core/models/oci.core.models.Subnet.html#oci.core.models.Subnet.availability_domain
| def __init__( | ||
| self, tag, timestamp_suffix=True, compartment_id=None, | ||
| config_path='~/.oci/config', | ||
| availability_domain=None, config_path='~/.oci/config', |
There was a problem hiding this comment.
I know this the default value here is probably to maintain backwards compatibility, but since this a necessary parameter now I think we should not have a default value for it.
Also, I think we should also update the oci example file to include that change as well
There was a problem hiding this comment.
Yeah, I have always been of the opinion that the common arguments between the clouds (tag and timestamp_suffix) should go first in __init__. Since timestamp_suffix has a default, everything after it would also have to have a default, including "required" arguments.
I know we already have another cloud or two that breaks this, but I was hoping to change it to be consistent, not the other way around. I fine doing whatever the team thinks is best though.
Thanks for the reminder about the example
There was a problem hiding this comment.
That's a good point about the consistency. Fair enough. I don't see a problem keeping it as it is to enforce that
2 participants
availability_domain used to be able to be inferred from the subnet.
Now it is required to be passed manually.
https://docs.oracle.com/en-us/iaas/tools/python/2.38.3/api/core/models/oci.core.models.Subnet.html#oci.core.models.Subnet.availability_domain
Fixes #138