Modified code to support python 2. #41
Conversation
rvelaVenafi
requested review from
angelmoo,
asquith-venafi,
tr1ck3r and
warrior-abhijit
| def __setattr__(self, key, value): | ||
| if key == "_base_url": | ||
| value = self._normalize_and_verify_base_url(value) | ||
| self.__dict__[key] = value |
There was a problem hiding this comment.
There is a handy Python wrapper called @Property. This can be handy here. It would look like this:
@Property
def base_url(self):
# This is a getter
return self._base_url
@base_url.setter
def base_url(self, value):
# This is the setter method
self._base_url = self._normalize_and_verify_base_url(value)
It's nicer for refactoring and is pretty explicit.
There was a problem hiding this comment.
Thanks for this suggestion @HELGAHR , we will look into it.
| @staticmethod | ||
| def _parse_zone_config_to_policy(data): | ||
| # todo: parse over values to regexps (dont forget tests!) | ||
| p = data["Policy"] |
There was a problem hiding this comment.
How safe is it in this method to assume that these dictionary keys resolve? I'm new to this code, but I usually think thrice before trying to access a node in the dictionary without .get().
There was a problem hiding this comment.
Thanks for this suggestion as well. Although the code itself was not added in this PR (it was present before), it is nice to have potentially dangerous code design choices pinpointed
| if re.match(r"^\\\\VED\\\\Policy", zone): | ||
| return zone | ||
| else: | ||
| if re.match(r"^\\\\", zone): |
There was a problem hiding this comment.
Just a tidbit of input: Python string objects have a .startswith() method that's easier to read than a regex, although a regex works fine.
| :param str url: | ||
| :param str user: | ||
| :param str password: | ||
| :param str access_token: | ||
| :param str refresh_token: | ||
| :param dict[str,Any] http_request_kwargs: |
There was a problem hiding this comment.
No use in having a doc string if the parameters aren't described, IMO.
| if key == "_base_url": | ||
| value = self._normalize_and_verify_base_url(value) | ||
| self.__dict__[key] = value |
There was a problem hiding this comment.
There is a Python wrapper called @Property. Its purpose is to define a getter, setter, and/or deleter method to an instance or class variable. It's nicer for refactoring, mostly. Worth noting if you're interested.
No description provided.