-
Notifications
You must be signed in to change notification settings - Fork 37
Conversation
Updated Account objects to be a generic type instead of using the DB schema directly. This allows for much easier adaptation to other non-AWS cloud providers. This change also modifies the DNS collector to use a new AXFR or CloudFlare account type object with the settings on the account object. This allows you to configure more than one AXFR and/or CloudFlare accounts
self.log.exception('Failed deleting account: {}'.format(self.id)) | ||
db.session.rollback() | ||
|
||
def to_json(self, is_admin=False): |
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.
This function name seems misleading. It's called to_json
but it returns a dict instead of a json string.
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.
I don't disagree, but this is a pattern we use for all types. We can look at refactoring this later, but its out of scope for this particular refactor.
Since we use a dynamic introspection pattern to convert objects to json encodable output we cannot rename the function in a single place, we'd have to do it for everything in one go.
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.
Okay thank you for the explanation. I'll create a ticket.
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.
return getattr(db, cls.__name__).find_one({ | ||
'account_id': account_id | ||
}) | ||
# class OldAccount(Model, BaseModelMixin): |
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.
Can you add a comment above this indicating why all this code is commented out?
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.
Should be deleted
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.
Removed old code and explained the to_json
name.
Updated Account objects to be a generic type instead of using the DB
schema directly. This allows for much easier adaptation to other non-AWS
cloud providers.
This change also modifies the DNS collector to use a
new AXFR or CloudFlare account type object with the settings on the
account object. This allows you to configure more than one AXFR and/or
CloudFlare accounts