Repository navigation
Sync nf-core joiners to Seqera workspace - #24
adamrtalbot wants to merge 4 commits into
Conversation
|
@FriederikeHanssen merge when stuff is set up |
|
This looks good, thanks! One addition I'd like is for it to remove users on sync as well. For example if we remove someone from the GitHub org id like them to also get removed from platform. Also is this going to spam people? I guess everyone who joins nf-core will get an invite email to platform? |
1. Removing users on sync There's no real "sync" in this PR yet. It's event-driven: a user is added only when
Matching on records the bot wrote avoids the hard part, which is mapping GitHub users to Seqera emails after the fact. 2. Spam It's limited to one call per person, at the moment they're invited (or when someone runs
Generated by Claude Code |
Suggestion: a dedicated The auto-sync covers new joiners. A separate command would let admins manage Seqera access directly, for corrections, for people whose Slack email differs from their Seqera email, and for groups other than megatests:
Slack messages don't yet reflect the Seqera step The PR hasn't changed since the first review (still the same 2 commits), so these haven't been edited yet:
Generated by Claude Code |
|
So we tried doing a sync via IAC (Alchemy) on nf-core/ops. It looked practical but most users had a different email between GitHub and Seqera Platform, resulting in us requiring a mapping file between most users which needs to be manually maintained, so the sync breaks down. This felt like the smallest MVP: add users with the view role to a workspace when they ask to join nf-core. This means every user is added to the workspace automatically, they can then be migrated to the maintainer or core teams by some downstream process. Spam wise, it's just every user receiving one extra email so I don't think it's a big deal. Adding a separate slash command would work, but doesn't really solve the core syncing problem. |
|
Let's incorporate the fixes, and make the process an add and sync for the users to the workspace. Follow up issue for the slash command. Do we already have a dynamoDB we can store info in? |
|
Summary of the changes for this PR:
The slash command is out of scope and tracked in #25. |
- Return a SeqeraResult with a reason (not configured, no email, no account, API error) instead of a bool; stay silent when unconfigured. - Add to the workspace before the welcome and mention megatests access; existing-member replies no longer say "nothing to do" after an add. - Use an optional Platform email from the GitHub Invitation Request form, falling back to the Slack email. - Record each add in DynamoDB (PK=SEQERA#<workspace> SK=USER#<slack-id>). - Weekly job removes recorded users with no active or pending GitHub org membership; log-only while SEQERA_REMOVAL_DRY_RUN is on (default). - Tests and docs for the above. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4T79rC8VmKnzeJDnnFnge
|
Pushed 24cc242, which covers all 8 items from the summary above. CI is green. Before merging:
Generated by Claude Code |
adamrtalbot
left a comment
There was a problem hiding this comment.
Thanks, this looks good overall. Two things before merging:
1. The Seqera SSM secrets are required, not optional (infra/stacks/bot_stack.py)
SEQERA_TOKEN, SEQERA_ORG_ID and SEQERA_WORKSPACE_ID are added to the ECS secrets list. If the stack is deployed before all three parameters exist in SSM, the task fails to start and the whole bot goes down, not just the Seqera step. docs/deployment.md says a missing parameter means "the step is skipped silently", which isn't true for the deployed bot. Please either fix the docs to say the parameters must exist before deploying, or make these secrets optional.
2. Check how the add response codes are mapped (checks/seqera.py)
- A 404 is treated as "no Seqera account", but the OpenAPI spec for
participants/adddoesn't list a 404. It lists only 200, 400, 403 and 409. An email with no account may come back as something else, such as a 400. Since most new joiners won't have a Seqera account, this path will be the common one. Could you test one add with an email that has no account and adjust the mapping to match? - The spec says a 409 means "Duplicate element or collaborator not permitted for organization with active SSO", but every 409 is reported as "already a participant". This only matters if the nf-core org uses SSO, so it's worth a quick check.
Keeping removal dry-run on for the first week or two, as the PR already suggests, sounds right.
Generated by Claude Code
- A 409 from participants/add also means "collaborator not permitted for organization with active SSO", so only report "already a participant" after a participant search finds the email; otherwise report the 409 and Seqera's message. - Include Seqera's own message in the "no account" (404) result, since that mapping isn't in the spec and is unconfirmed. - Docs: the SEQERA_* SSM parameters are ECS secrets and must exist before deploying, or the task fails to start. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4T79rC8VmKnzeJDnnFnge
|
Addressed in e716614:
Generated by Claude Code |
|
@ewels what do you think of this now? |
Why
New nf-core joiners should get read-only access to the megatests workspace without a separate manual step, and should lose it when they leave the nf-core GitHub org.
What
nf-core/megatestsSeqera workspace with thenf-core-read-onlyrole, for new GitHub invitees and for people already in the org.SEQERA_*params aren't set.PK=SEQERA#<workspace-id> SK=USER#<slack-user-id>: GitHub username, email used, participant ID).scheduler/seqera_jobs.py, Mondays 09:00 UTC): removes recorded users who have neither an active membership nor a pending invite in the nf-core GitHub org. It only touches people the bot added, and only that workspace.SEQERA_REMOVAL_DRY_RUN(defaulttrue) logs who would be removed without removing anyone.docs/deployment.md.The slash command is tracked in #25.
🤖 Generated with Claude Code
https://claude.ai/code/session_01A4T79rC8VmKnzeJDnnFnge