-
Notifications
You must be signed in to change notification settings - Fork 13
Feature/api clients #389
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
Feature/api clients #389
Conversation
@disable_ssl_option | ||
@use_v2_file_events_option | ||
@debug_option | ||
def create_api_client( |
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 chose to make this a separate create
command for a couple reasons
- py42's sdk uses two different auth methods
- the
totp
option isn't applicable with api-client auth - I didn't think an optional password/secret (and in turn prompting the user for authentication on every command) made sense in the case of api clients.
- allowing users to pass in secret and password interchangeably makes it difficult to understand what the users trying to do in the case of the
code42 profile update
command
Whether or not this should be a separate command or if we should just add options to the original create
command (which is how the ticket was initially authored) is definitely open for discussion.
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.
yeah, a separate command makes sense, given there's enough differences. Making it explicit makes it easier to document and harder for a user to do something wrong unintentionally.
@username_option() | ||
@password_option | ||
@totp_option | ||
@disable_ssl_option | ||
@use_v2_file_events_option | ||
@yes_option(hidden=True) | ||
@debug_option | ||
def update( |
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.
If this method is now too unsightly for mortal eyes I could move the param error checking into a helper function
@@ -105,24 +120,6 @@ def delete_profile(self, name): | |||
self._internal[self.DEFAULT_PROFILE] = self.DEFAULT_VALUE | |||
self._save() | |||
|
|||
def _set_authority_url(self, new_value, profile): |
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.
simplified these one liners
a6afc15
to
8148859
Compare
8148859
to
bd06572
Compare
] | ||
for matter in matters: | ||
try: | ||
matter["creator_username"] = matter["creator"]["user"]["email"] |
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 will fail if the matter was created via an API client, right?
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'm not sure what the conditions for the field being populated are (but that would make sense). This creator user email
field isn't even included on the response documentation and isn't on every matter, but it does exist and is populated on some of them.
Adds API client support
code42 profile create-api-client
command--secret
and--api-client-id
options tocode42 profile update
code42 legal-hold list
andcode42 legal-hold show