Skip to content

Add workspace creation option to copy + new migrate-users command#20

Open
chasefortier wants to merge 9 commits intomainfrom
chasefortier/migrate-workspaces-and-users
Open

Add workspace creation option to copy + new migrate-users command#20
chasefortier wants to merge 9 commits intomainfrom
chasefortier/migrate-workspaces-and-users

Conversation

@chasefortier
Copy link
Copy Markdown
Contributor

Currently cometx can help migrate data, but does NOT migrate user permissions or workspaces, which can require manual effort if doing a migration. I have made the below updates to address this:

  • cometx copy --create-workspaces: Added a --create-workspaces flag to the copy command. When the destination workspace does not exist, cometx will attempt to create it via the Comet REST API (POST /api/rest/v2/write/workspace/new). If creation fails (e.g., insufficient permissions), a clear error message is shown. Without the flag, the existing behavior is preserved — an error is raised suggesting the user pass --create-workspaces or create the workspace via the UI.

  • cometx migrate-users: New command to migrate workspace membership from a source Comet environment to a destination environment. It reads a chargeback report (fetched from the source admin API or provided as a local JSON file) and invites each user to the corresponding workspace in the destination by email. Supports:

--create-workspaces to auto-create missing workspaces on the destination before adding members
--dry-run to preview all operations without making changes
--chargeback-report to use a pre-downloaded report file instead of fetching from the source
Detailed per-workspace and summary logging of added, skipped, already-member, and failed users
Failures are saved to bulk_add_failures_by_email.json for follow-up

Tests Completed
[ ] Run cometx copy SOURCE DEST without the flag when workspace doesn't exist — verify it errors with a message suggesting --create-workspaces
[ ] Run cometx copy SOURCE DEST --create-workspaces when workspace doesn't exist — verify it creates the workspace and proceeds
[ ] Run cometx copy SOURCE DEST --create-workspaces when workspace already exists — verify it proceeds normally
[ ] Run cometx migrate-users --api-key KEY --source-api-key KEY --dry-run — verify it previews without making changes
[ ] Run cometx migrate-users --api-key KEY --chargeback-report /path/to/report.json — verify it loads from file
[ ] Run cometx migrate-users --api-key KEY --source-api-key KEY --create-workspaces — verify workspaces are created before adding members
[ ] Verify failures are written to bulk_add_failures_by_email.json
[ ] Run cometx migrate-users --help and cometx copy --help to verify argument documentation

Creating a new cometx/cli/migrate_users.py module, which takes a chargeback report from a source environment, and invites users to all of the same workspaces in a destination environment

Adding --create-workspaces argument to both copy and migreate_users so user can decide whether to create a workspace when it doesn't exist.
Creating a new cometx/cli/migrate_users.py module, which takes a chargeback report from a source environment, and invites users to all of the same workspaces in a destination environment

Adding --create-workspaces argument to both copy and migreate_users so user can decide whether to create a workspace when it doesn't exist.
@dsblank dsblank force-pushed the chasefortier/migrate-workspaces-and-users branch from d609047 to 7cd5e4b Compare March 13, 2026 17:14
@chasefortier
Copy link
Copy Markdown
Contributor Author

chasefortier commented Mar 25, 2026

Hey @dsblank any chance you could do a final review of this in the next day or two? A customer has a need to migrate users, so ideally could merge in these changes with the new migrate-users command!

chasefortier and others added 5 commits March 25, 2026 13:18
update admin=True in the user invite payload, ensuring that the user is added as a workspace owner rather than just a member
* Fix review issues in migrate-users PR

- copy.py: move `import requests` to top-level; use `self.api._get_url_server()`
  instead of broken `self.api.server_url`; split HTTPError vs RequestException
  handlers for precise error messages
- migrate_users.py: add --url/--source-url args for self-hosted instances;
  fix _resolve_server_url (base64 padding, error handling, explicit-URL priority);
  require --source-api-key explicitly instead of silently reusing COMET_API_KEY;
  warn when source and dest URL+key are identical; handle dict-list API response
  in _get_existing_workspaces; show per-user output in dry-run; add
  --failures-output flag; use json= kwarg in _add_member; fix docstring examples
- tests/unit/test_migrate_users.py: 17 new unit tests covering _resolve_server_url,
  _get_existing_workspaces, _add_member, and dry-run behaviour

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Address PR #21 review comments

- Fix duplicate migrate-users subparser registration in __init__.py
- Validate explicit URLs require https:// in _resolve_server_url (SSRF fix)
- Remove silent COMET_CLOUD_URL fallback; fail fast when URL cannot be determined
- Handle {"workspaceNames": [...]} dict response shape in _get_existing_workspaces
- Deduplicate workspace creation by reusing _create_workspace from migrate_users in copy.py
- Update tests to match new behavior (SystemExit instead of COMET_CLOUD_URL fallback, new dict response shapes)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Douglas Blank <doug@comet.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
- Drop dead COMET_CLOUD_URL constant left over after silent-fallback removal
- Replace hand-rolled assertRaises context manager with pytest.raises

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@dsblank dsblank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small fixes already pushed to the branch:

  • Removed the dead COMET_CLOUD_URL constant left over after the silent-fallback removal
  • Replaced the hand-rolled assertRaises context manager in the test with pytest.raises

One question that needs an answer before merge:

_add_member always passes "admin": True (migrate_users.py, _add_member function):

payload = {
    "userEmail": email,
    "workspaceName": workspace_name,
    "admin": True,
}

This adds every migrated user as a workspace admin regardless of their role in the source environment. The chargeback report members objects may contain role information. Was this intentional — i.e., the migration is specifically for workspace owners/admins only — or should the role be read from the report? If it's intentional, a comment explaining why would help future readers.

@chasefortier
Copy link
Copy Markdown
Contributor Author

@dsblank looks like chargeback reports actually do not contain info on user workspace role.

The main permissions that workspace ownership gives a user is the ability to add additional users - I would like to default to setting users to owners so that they can invite colleagues! Otherwise will cause friction post migration.

@dsblank
Copy link
Copy Markdown
Collaborator

dsblank commented Mar 27, 2026

@chasefortier said:

The main permissions that workspace ownership gives a user is the ability to add additional users - I would like to default to setting users to owners so that they can invite colleagues!

Is "setting users to owners" the same as the current status:

payload = {
    "userEmail": email,
    "workspaceName": workspace_name,
    "admin": True,
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants