-
Notifications
You must be signed in to change notification settings - Fork 133
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
Kasiah/empower connector #1191
Kasiah/empower connector #1191
Conversation
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.
Lots of comments but nothing major, excited to get this merged soon
|
||
return [ctas, cta_prompts, cta_prompt_answers] | ||
|
||
def get_ctas(self): |
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.
Enough data restructuring is happening in these methods (or the private/utility methods they're calling) that it would be useful for each of the dcostrings for the public methods to describe the format of the data being returned.
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.
One small additional change requested (re: the unix timestamp) but otherwise this looks good enough. I do wish there was a bit more documentation of how the methods are constructed and what they're doing to the data but I understand since you didn't write the code yourself that's hard to provide.
Branched off @jburchard 's Empower connector PR.
Adds a single method to return the full export unparsed, for pure ELT.