Skip to content

(INF-2987) Create script to mass import users into CO Manage Registry from files#29

Open
williamnswanson wants to merge 16 commits into
masterfrom
INF-2987.mass-person-create-update-script
Open

(INF-2987) Create script to mass import users into CO Manage Registry from files#29
williamnswanson wants to merge 16 commits into
masterfrom
INF-2987.mass-person-create-update-script

Conversation

@williamnswanson

Copy link
Copy Markdown
Collaborator

Takes a file with a list of maps of key:value pairs and creates a CO Person record in the COmanage Registry (along with CO Roles, CO Group Memberships, SSH Keys, Unix cluster accounts and groups, ect)

The script doesn't currently have a way to know what parts of the mapping file should be used for what attributes of what type of record, so the build_co_person_record() function will need to be modified to suit the specific import you're trying to do, what data you have for the users, ect.

@williamnswanson williamnswanson self-assigned this Jan 30, 2026
@williamnswanson williamnswanson added the enhancement New feature or request label Jan 30, 2026
@brianhlin
brianhlin requested review from jasoncpatton and removed request for brianhlin March 17, 2026 17:47
@brianhlin
brianhlin requested a review from Copilot June 17, 2026 19:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a new bulk-import script to create COmanage Registry CO Person records from JSON input, including related objects (roles, group memberships, SSH keys, Unix cluster accounts/groups), supported by new schema-building helpers and some new COmanage utility APIs.

Changes:

  • Added mass_person_create_modify.py to read an input data file + mapping file and create/provision CO Person-related records in COmanage.
  • Added comanage_person_schema_utils.py with helpers for constructing Core API v1-style CO Person payloads.
  • Extended comanage_utils.py with Core API v1 people helpers, new retry/fast-fail HTTP handling, and group creation helper.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 7 comments.

File Description
mass_person_create_modify.py New mass import script to create CO People, groups, Unix accounts, and provision them.
comanage_utils.py Adds HTTP fast-fail exception type, Core API people helpers, identifier update helper, and CO group creation helper.
comanage_person_schema_utils.py New module defining payload templates and helper constructors for CO Person-related objects.
.gitignore Ignores Python __pycache__ artifacts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread comanage_utils.py Outdated
Comment thread comanage_utils.py Outdated
Comment thread comanage_utils.py Outdated
Comment thread comanage_person_schema_utils.py Outdated
Comment thread mass_person_create_modify.py
Comment thread mass_person_create_modify.py
Comment thread mass_person_create_modify.py
@williamnswanson

Copy link
Copy Markdown
Collaborator Author

While we're in the mood of doing LLM-reviews, I had Claude take a look:
comanage_import_code_review.md

I gave it the Import script, comanage_utils.py and the schema helper (as well as some links to COmanage Docs and some info on Identifier Assignment), so it's scope is a bit larger than just this PR.

Honestly, it's a little brutal 😅?

High level Headers / topics {with notes}

Critical: (potential) Run stoppers

  • C1. Dependency on synchronous Identifier Assignment {osgid and uid assignment, we assume it works and I don't think there's ever a case where they get added "late"}

  • C2. OrgIdentity reuses the CoPerson identifier list (shared refs + likely uniqueness violation)

  • C3. Partially-created users are never repaired on re-run {IIRC, The general philosophy was halting everything on any sort of failure to avoid having too many cases to fix? With there not being enough time to make the script "smart" enough to meaningfully attempt fixes}

High: wrong behavior or fragile control flow

  • H4. Inconsistent next() guarding across the two Unix helpers

  • H5. One non-404 read error aborts the whole batch {See C3 note about fail-fast}

  • H6. Status value inconsistency: "A" vs "Active" {I think the confusion here is that COmanages API is actually looking for just "A"?}

  • H7. Double provision_group, and unverified CoPerson provisioning {I think the double-provision was just for "safety"? CO Person gets provisioned by COmanage once the Unix Cluster Account gets made IIRC}

Medium: robustness and modern best practices

  • M8. Hand-rolled urllib client → use requests/httpx + a real retry policy {Old inherited code that we didn't want to touch for every COmanage-facing thing. But still prob good advice.}

  • M9. Backoff doubles as the socket timeout; no cap, no jitter {Some jank we (I) kind of just threw together over the years? }

  • M10. URL construction is not URL-safe {We're using "os.path.join" for url paths... Works fine on linux! (until it doesn't) 😓 }

  • M11. GET requests send a "null" body {Works fine for COmanage 🤷}

  • M12. Leaked file handles and unclosed responses {Should be using "with"s for auto-closing}

  • M13. getopt + print → argparse + logging {This is known 😔}

  • M14. Two divergent identifier schemas coexist in the library {Documenting which code paths use the v1 API vs the CORE API}

Low — cleanliness and small correctness nits {Left of of this overview, but there aren't many}

@brianhlin @jasoncpatton Any thoughts? Might give it a link to the COmanage Github repo to see if it can make anything of it.
(If you want to see the whole (short) convo I can paste it somewhere.)

@brianhlin

Copy link
Copy Markdown
Member

@williamnswanson yeah my gut is that this scriptery isn't invoked enough to be worth spending much effort on. But having Claude make attempts at fixing the various issues might be a decent exercise in of itself?

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants