-
Notifications
You must be signed in to change notification settings - Fork 0
LFXV2-867, LFXV2-877: Repository restructuring and platform users table sync implementation #17
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
Conversation
- Move go.mod/go.sum from v1-sync-helper/ to repo root - Move v1-sync-helper/ to cmd/lfx-v1-sync-helper/ following Go conventions - Rename cmd directory to include lfx- prefix for consistent image naming - Update all Go files to use standardized module documentation - Move Dockerfile to docker/ directory with descriptive name - Move Makefile to repo root and update for new structure - Update GitHub Actions workflows to use new paths - Configure ko build to maintain same container image name - Update binary names and build targets for consistency - Create comprehensive .dockerignore at repo root - Update .gitignore to include Go build artifacts and PEM files - Update README links to reflect new structure - Remove duplicate .gitignore/.dockerignore from cmd directory 🤖 Generated with GitHub Copilot (via Zed) Signed-off-by: Eric Searcy <[email protected]>
…bles - Add salesforce.merged_user table with specific column selection to Meltano configuration - Add salesforce.alternate_email__c table with all columns to Meltano configuration - Add handler for alternate email updates with concurrency-controlled v1-mapping maintenance - Implement atomic KV operations with retry logic and random splay time for collision avoidance - Create v1-merged-user.alternate-emails mapping records to track user email relationships 🤖 Generated with [GitHub Copilot](https://github.com/features/copilot) (via Zed) Signed-off-by: Eric Searcy <[email protected]>
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.
Pull request overview
This pull request implements two significant improvements to the lfx-v1-sync-helper service: repository restructuring for consistency with other LFX services (LFXV2-867) and implementation of Salesforce merged user table synchronization (LFXV2-877).
The repository restructuring moves Go modules to the root level and relocates service code to cmd/lfx-v1-sync-helper/, aligning with Go best practices and enabling proper ko image naming that matches Helm chart conventions. This improves maintainability and follows the standard structure used across LFX services.
The Salesforce table sync implementation adds support for synchronizing merged_user and alternate_email__c tables through Meltano, with a new Go handler that maintains alternate email mappings using atomic NATS KV operations. The implementation includes retry logic with random splay to handle concurrent updates safely in multi-pod Kubernetes deployments.
Key changes:
- Restructured repository with Go modules at root and service code in
cmd/lfx-v1-sync-helper/ - Updated build tooling (Makefile, Dockerfile, GitHub Actions) to reflect new structure
- Added Meltano configuration for Salesforce
merged_userandalternate_email__ctable sync - Implemented concurrent-safe handler for alternate email mapping with atomic KV operations and retry logic
Reviewed changes
Copilot reviewed 25 out of 31 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | New Go module file at repository root defining dependencies (moved from subdirectory) |
| go.sum | New Go module checksums at repository root |
| cmd/lfx-v1-sync-helper/handlers.go | Added handleAlternateEmailUpdate with retry logic and atomic KV operations for alternate email mapping |
| cmd/lfx-v1-sync-helper/nats_client.go | New file with NATS client helper functions for project UID/slug lookups |
| cmd/lfx-v1-sync-helper/*.go | Updated package comments to use consistent "lfx-v1-sync-helper" naming |
| cmd/lfx-v1-sync-helper/README.md | New comprehensive service documentation at updated location |
| meltano/meltano.yml | Added Salesforce merged_user and alternate_email__c table configurations with incremental replication |
| docker/Dockerfile.v1-sync-helper | Updated Docker build to reference cmd/lfx-v1-sync-helper and new binary name |
| Makefile | Updated build targets to reference cmd/lfx-v1-sync-helper path and support .env file for Docker |
| .github/workflows/*.yaml | Updated GitHub Actions to reference go.mod at root and build from cmd/lfx-v1-sync-helper |
| .gitignore | Added Go build artifacts (/bin/) and *.pem to root gitignore |
| .dockerignore | New comprehensive Docker ignore file at repository root |
| .yamllint.yml | Updated YAML linting rules to accommodate Meltano formatting |
| charts/lfx-v1-sync-helper/values.yaml | Minor formatting cleanup (whitespace alignment in comments) |
| charts/lfx-v1-sync-helper/README.md | Updated documentation link to point to new cmd/lfx-v1-sync-helper location |
| README.md | Updated service documentation link to reflect new structure |
| v1-sync-helper/.gitignore | Removed (consolidated into root .gitignore) |
| v1-sync-helper/.dockerignore | Removed (consolidated into root .dockerignore) |
| v1-sync-helper/handlers.go | Removed (moved to cmd/lfx-v1-sync-helper/) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Move handleAlternateEmailUpdate and related functions from handlers.go to handlers_users.go - Update module docstring to match consistent pattern across handler files - Maintain same package structure and function visibility for proper integration - Follow established pattern of separating handlers by domain (users, projects, committees, meetings) 🤖 Generated with [GitHub Copilot](https://github.com/features/copilot) (via Zed) Signed-off-by: Eric Searcy <[email protected]>
Change JoinTime from time.Time to *time.Time with omitempty tag to allow for cases where join time might not be available or recorded. This change was missed in PR #16. 🤖 Generated with GitHub Copilot (https://github.com/features/copilot) (via Zed) Signed-off-by: Eric Searcy <[email protected]>
- Remove unnecessary else block after return statement in handlers_users.go - Fix dead link to root README.md by updating relative path 🤖 Generated with [GitHub Copilot](https://github.com/features/copilot) (via Zed) Signed-off-by: Eric Searcy <[email protected]>
…eneration - Use context.WithoutCancel to prevent goroutine cancellation while preserving context values - Switch from math/rand to math/rand/v2 for better performance and modern API 🤖 Generated with [GitHub Copilot](https://github.com/features/copilot) (via Zed) Signed-off-by: Eric Searcy <[email protected]>
This PR addresses two key improvements to the lfx-v1-sync-helper service:
LFXV2-867: Repository Restructuring
Summary: Restructured the repository to follow modern Go module and containerization best practices, improving maintainability and ensuring consistency with other LFX services.
Key Changes:
go.modandgo.sumto repository rootcmd/lfx-v1-sync-helper/for proper ko image namingdocker/Dockerfile.v1-sync-helper.gitignoreand.dockerignorefiles from subdirectories.envfile support for secure local Docker configurationBenefits:
ghcr.io/linuxfoundation/lfx-v1-sync-helper/lfx-v1-sync-helper)LFXV2-877: Salesforce Merged User Table Sync
Summary: Implemented data synchronization for v1 Platform (PostgreSQL)
merged_userandalternate_email__ctables with robust concurrent-safe mapping logic.Key Changes:
salesforce.merged_user(subset of columns) andsalesforce.alternate_email__c(all columns)lastmodifieddatefieldsalesforce-alternate_email__ckey prefixv1-merged-user.alternate-emailsto track alternate email SFIDs byleadorcontactidTechnical Details:
tap-postgresfor table extractiontarget-nats-kvas the data loaderBenefits:
Testing
Related Issues
🤖 Generated with GitHub Copilot (via Zed)