[LIBCLOUD-747] Add Zonomi DNS provider implementation #582
Conversation
aleGpereira
force-pushed
the
LIBCLOUD-747_zonomi_provider
branch
from
c9cf79b
to
028ed3f
Compare
|
Thanks for all those PRs and sorry for the delay (I've been traveling and was offline). I will have a look as soon as the time permits. |
|
It's fine. I have another one for monday or tuesday. That's the last one. Thank you guys for your awesome work! |
aleGpereira
force-pushed
the
LIBCLOUD-747_zonomi_provider
branch
2 times, most recently
from
479b013
to
869ef31
Compare
aleGpereira
force-pushed
the
LIBCLOUD-747_zonomi_provider
branch
from
869ef31
to
ff76549
Compare
| self.status = response.status | ||
|
|
||
| # This attribute is used when using LoggingConnection | ||
| original_data = getattr(response, '_original_data', None) |
There was a problem hiding this comment.
I noticed you duplicated this code from the base class in one other driver as well. I know there is a small difference at the bottom, but duplication like this is problematic (it makes code hard to maintain). On top of the duplication, this also break the implicit signature / contract of the parse_body method (it should return an object, not a tuple).
I will review this and merge this today, but after this is merged can you please open a new PR which reduces the duplication - you should re-use as much functionality from the parent class as possible.
One thing you could do is make parse_body a no-op and add a new parse_body_and_errors method and make constructor look something like this:
def __init__(self, response, connection):
super(Foo, self).__init__(response=response, connection=connection)
self.objects, self.errors = self.parse_body_and_errors()
if self.errors:
raise self._make_excp(self.errors[0])There was a problem hiding this comment.
Sure. Sorry for my bads! I'll look at this tomorrow. Thank you so much!
|
Merged into trunk, thanks! When time permits, please open a new PR (also for the other driver) which addresses this issue - https://github.com/apache/libcloud/pull/582/files#r40497873 |
No description provided.