Add databricks scim command to expose the SCIM api: #311
Conversation
…development Fix delete method to only return an empty dict if there was no body in the response.
| @@ -62,6 +63,7 @@ def cli(): | |||
| cli.add_command(secrets_group, name='secrets') | |||
| cli.add_command(stack_group, name='stack') | |||
| cli.add_command(groups_group, name='groups') | |||
| cli.add_command(scim_group, name='scim') | |||
There was a problem hiding this comment.
can you split scim into two commands - users & groups (groups_group can also be changed). for end users this is what would make the most sense, not SCIM (which is heavily enterprise-admin focused).
|
|
||
|
|
||
| class ScimApi(object): | ||
| GROUP_NAME_FILTER = 'displayName eq {}' |
There was a problem hiding this comment.
if group name got spaces in it, this condition will fail. please quote it
| filters=self.GROUP_NAME_FILTER.format(group_name), | ||
| data=content) | ||
|
|
||
| def get_group_name_for_group(self, group_id): |
There was a problem hiding this comment.
change to get_group_name_by_id
| content = self.list_users(filters=filters) | ||
| return content | ||
|
|
||
| def get_user_id_for_user(self, user_name): |
There was a problem hiding this comment.
change name to get_user_id_by_name
| filters=self.USER_NAME_FILTER.format(user_name), | ||
| data=content) | ||
|
|
||
| def get_group_id_for_group(self, group_name): |
There was a problem hiding this comment.
change to get_group_id_by_name
|
|
||
|
|
||
| @click.command(context_settings=CONTEXT_SETTINGS, | ||
| short_help='NOT IMPLEMENTED') |
| @click.command(context_settings=CONTEXT_SETTINGS, | ||
| short_help='List groups') | ||
| @click.option('--filters', default=None, required=False, help=FILTERS_HELP) | ||
| @click.option('--group-id', metavar='<int>', cls=OptionalOneOfOption, default=None, |
There was a problem hiding this comment.
why not --id and --name - it's already clear from command name it's group or user.
| @click.option('--group-name', required=True) | ||
| @click.option('--member', 'members', is_flag=False, default=None, metavar='<members>', | ||
| type=click.STRING, | ||
| help='members to add to the group', multiple=True, required=False) |
There was a problem hiding this comment.
add more docs here - is it --member user.name@example.com --member user2@example.com or it is --member id --member id2.
TBD:
it's preferable to have databricks groups create "Data Science" --user-name ben@example.com --user-name john@example.com --group-name "Data Interns"
| @eat_exceptions | ||
| @provide_api_client | ||
| # pylint:disable=unused-argument | ||
| def groups_update_cli(api_client, group_id, operation, path, values): |
There was a problem hiding this comment.
add more semantic commands here. see comment about users.
There was a problem hiding this comment.
See change requests in context of specific methods.
Please also add 85% unit test coverage for new code to match quality assurance criteria for other code in this repo - https://codecov.io/gh/databricks/databricks-cli/tree/master/databricks_cli
Codecov Report
@@ Coverage Diff @@
## master #311 +/- ##
=========================================
Coverage ? 81.64%
=========================================
Files ? 41
Lines ? 3083
Branches ? 0
=========================================
Hits ? 2517
Misses ? 566
Partials ? 0
Continue to review full report at Codecov.
|
|
FYI: This is stale, it has semi-useful code but has a lot to be addressed. |
https://docs.databricks.com/dev-tools/api/latest/scim/index.html