-
Notifications
You must be signed in to change notification settings - Fork 45
Unicron signatures per month #4878
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
Signed-off-by: Lukasz Gryglicki <[email protected]> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <[email protected]> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughA new monthly signature report program is introduced that scans the CLA DynamoDB signatures table, aggregates signed and approved signatures by month, classifies them as ICLA, ECLA, or CCLA, and writes the results to a semicolon-delimited CSV file. Changes
Sequence DiagramsequenceDiagram
participant Main as main()
participant AWS as AWS Session
participant DDB as DynamoDB
participant Parser as Date & Type Parser
participant Aggregator as Monthly Aggregator
participant CSV as CSV Writer
Main->>AWS: Create session with profile & region
AWS-->>Main: Session established
Main->>DDB: Scan cla-prod-signatures table
loop For each batch of items
DDB-->>Main: Return signature records
Main->>Parser: Extract & validate date, classify type
alt Invalid/Future date
Parser-->>Main: Skip record (log count)
else Valid date
Parser-->>Main: Return month & signature class
Main->>Aggregator: Add to monthly counts
end
Main->>Main: Log progress (every 1000 records)
end
Main->>Aggregator: Sort monthly records
Aggregator-->>Main: Sorted slice
Main->>CSV: Write headers & sorted records
CSV-->>Main: Report file created
Main->>Main: Output summary totals & skip counts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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 PR adds a monthly signature report generator that scans a DynamoDB table to produce statistics on ICLA, ECLA, and CCLA signatures by month. The script generates a CSV file containing signature counts from February 2019 through November 2025.
- Implements a DynamoDB scanner with projection expressions for efficient data retrieval
- Classifies signatures using both
sigtype_signed_approved_id(primary) andsignature_type(fallback) fields - Handles multiple date formats with comprehensive parsing logic
- Filters signatures by approval/signing status and validates against future dates
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cla-backend-go/monthly_signature_report.go | New standalone script that scans DynamoDB signatures table and generates monthly statistics by signature type (ICLA/ECLA/CCLA) |
| cla-backend-go/signature_monthly_report.csv | Generated CSV report containing 83 months of signature statistics with semicolon-delimited format |
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.
Actionable comments posted: 3
🧹 Nitpick comments (3)
cla-backend-go/monthly_signature_report.go (3)
47-59: Consider adding a context with timeout for long-running operations.A full table scan on a production DynamoDB table could take a significant amount of time. Without a context, the program could hang indefinitely if there are network issues.
+import "context" + func main() { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute) + defer cancel() + // Set up AWS session sess, err := session.NewSessionWithOptions(session.Options{ Profile: profileName, Config: aws.Config{ Region: aws.String(regionDefault), }, })Then use
svc.ScanPagesWithContext(ctx, params, ...)instead ofsvc.ScanPages.
227-231: Non-standard semicolon separator in CSV file.Using semicolon as the delimiter creates a semicolon-separated file rather than a standard CSV. While this works for some European locales and applications, it may cause import issues with tools expecting comma-delimited files. If this is intentional, consider renaming the output or adding a comment explaining the choice.
Is the semicolon separator intentional? If so, consider documenting it:
writer := csv.NewWriter(file) - // Set semicolon as separator + // Use semicolon as separator for European locale compatibility / Google Sheets import writer.Comma = ';'
338-344: Redundant month validation.This validation is unnecessary because
time.Parseguarantees a valid month (1-12) upon successful parsing. The checktestTime.Month() >= 1 && testTime.Month() <= 12will always be true. Similarly, the checks at lines 316 and 332 for month bounds are redundant.result := t.Format("2006-01") - - // Additional validation: don't return invalid months like 2025-26 - if testTime, testErr := time.Parse("2006-01", result); testErr == nil { - // Ensure the month is valid - if testTime.Month() >= 1 && testTime.Month() <= 12 { - return result - } - } - - return "" + return result }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
cla-backend-go/signature_monthly_report.csvis excluded by!**/*.csv
📒 Files selected for processing (1)
cla-backend-go/monthly_signature_report.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cla-backend-go/monthly_signature_report.go (1)
cla-backend-go/signatures/models.go (2)
ICLA(81-81)CCLA(79-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: CodeQL analysis (go)
- GitHub Check: Agent
- GitHub Check: build-test-lint
- GitHub Check: cypress-functional
🔇 Additional comments (1)
cla-backend-go/monthly_signature_report.go (1)
28-45: LGTM!Struct definitions are well-organized with proper DynamoDB attribute tags.
Signed-off-by: Lukasz Gryglicki <[email protected]> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cla-backend-go/cmd/monthly_signature_report/monthly_signature_report.go (1)
339-347: Optional: Remove redundant validation.This validation re-parses the already-formatted string to check the month range. Since
time.Parse()at line 287 andtime.Format()at line 337 already guarantee a valid "2006-01" format with valid month (1-12), and lines 328-336 already validated the year and month ranges from the parsedtime.Timeobject, this additional check is unnecessary.Apply this diff to remove the redundant validation:
result := t.Format("2006-01") - - // Additional validation: don't return invalid months like 2025-26 - if testTime, testErr := time.Parse("2006-01", result); testErr == nil { - // Ensure the month is valid - if testTime.Month() >= 1 && testTime.Month() <= 12 { - return result - } - } - - return "" + return result }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
cla-backend-go/cmd/monthly_signature_report/monthly_signature_report.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-25T11:41:04.536Z
Learnt from: CR
Repo: linuxfoundation/easycla PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:41:04.536Z
Learning: When adding or updating tests, execute the new or modified tests to confirm they pass and ensure coverage reflects the newly added logic
Applied to files:
cla-backend-go/cmd/monthly_signature_report/monthly_signature_report.go
🧬 Code graph analysis (1)
cla-backend-go/cmd/monthly_signature_report/monthly_signature_report.go (1)
cla-backend-go/signatures/models.go (2)
ICLA(81-81)CCLA(79-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test-lint
- GitHub Check: cypress-functional
🔇 Additional comments (3)
cla-backend-go/cmd/monthly_signature_report/monthly_signature_report.go (3)
28-45: LGTM!The data structures are well-defined with proper DynamoDB attribute mappings. The struct design clearly separates the input record format from the aggregated statistics output.
100-196: The scan and classification logic is functionally correct.The signature type detection correctly handles both modern records (via
sigtype_signed_approved_idprefix) and legacy records (viasignature_typefield), with appropriate filtering for signed/approved status, valid dates, and future dates. Each signature is counted once in grand totals and once in monthly breakdowns, ensuring the monthly aggregations sum correctly to the totals.
210-257: LGTM!The CSV generation properly sorts monthly data chronologically (YYYY-MM string comparison works correctly), uses the specified semicolon delimiter, and includes comprehensive error handling for all file operations.
cc @mlehotskylf @ahmedomosanya - monthly report of ICLAs/ECLAs/CCLAs.
Data available here.
Signed-off-by: Lukasz Gryglicki [email protected]
Assisted by OpenAI
Assisted by GitHub Copilot