Update data sync to include all contributions within last 365 days #3232
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughSwitches GitHub sync defaults and lookback windows to a rolling 365-day range: CLI defaults now compute start = now(UTC) - 365 days and end = now(UTC); filtering for milestones, issues, and pull requests uses a 365-day fallback instead of shorter or fixed-calendar defaults. Tests updated accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-12-31T05:17:39.659ZApplied to files:
📚 Learning: 2026-01-01T17:48:23.963ZApplied to files:
🧬 Code graph analysis (1)backend/tests/apps/github/common_test.py (3)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/apps/owasp/management/commands/owasp_create_member_snapshot.py (1)
90-90: Remove redundant local imports of timedelta.Since
timedeltais now imported at the module level (line 5), these local imports within the methods are redundant.♻️ Proposed fix
Remove line 90:
- from datetime import timedelta - # Initialize all dates in range with 0Remove line 253:
- from datetime import timedelta - # Initialize all dates in range with 0Also applies to: 253-253
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/apps/github/management/commands/github_sync_user.pybackend/apps/owasp/management/commands/owasp_create_member_snapshot.pybackend/tests/apps/github/management/commands/github_sync_user_test.py
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/apps/owasp/management/commands/owasp_create_member_snapshot.pybackend/tests/apps/github/management/commands/github_sync_user_test.pybackend/apps/github/management/commands/github_sync_user.py
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/apps/owasp/management/commands/owasp_create_member_snapshot.pybackend/tests/apps/github/management/commands/github_sync_user_test.pybackend/apps/github/management/commands/github_sync_user.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/apps/owasp/management/commands/owasp_create_member_snapshot.pybackend/tests/apps/github/management/commands/github_sync_user_test.pybackend/apps/github/management/commands/github_sync_user.py
📚 Learning: 2026-01-01T18:57:05.007Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/video.py:189-215
Timestamp: 2026-01-01T18:57:05.007Z
Learning: In the OWASP backend area, maintain the established pattern: when dealing with sponsors, include all entries from Sponsor.objects.all() (including NOT_SPONSOR) and perform in-memory sorting using the same criteria/pattern used by the GraphQL sponsor query implemented in backend/apps/owasp/api/internal/queries/sponsor.py. Apply this behavior consistently to files in backend/apps/owasp (not just video.py), and ensure code paths that render sponsor lists follow this in-code sorting approach rather than pre-filtering NOT_SPONSOR entries before sorting.
Applied to files:
backend/apps/owasp/management/commands/owasp_create_member_snapshot.py
🔇 Additional comments (7)
backend/tests/apps/github/management/commands/github_sync_user_test.py (1)
158-158: LGTM! Test expectations correctly updated.The test assertions now verify that the help text reflects the new rolling 365-day window defaults instead of fixed calendar dates.
Also applies to: 163-163
backend/apps/owasp/management/commands/owasp_create_member_snapshot.py (3)
5-5: LGTM! Import addition supports rolling 365-day window.The
timedeltaimport is necessary for the new default date range calculation.
44-44: LGTM! Help text accurately reflects the new defaults.The updated help text clearly communicates the rolling 365-day window behavior to users.
Also applies to: 49-49
323-325: LGTM! Default date range correctly implements rolling 365-day window.The logic properly calculates the start date as 365 days before the end date (today), which aligns with the PR objective to capture all contributions from the last year.
backend/apps/github/management/commands/github_sync_user.py (3)
4-4: LGTM! Import addition supports rolling 365-day window.The
timedeltaimport is necessary for the new default date range calculation.
42-42: LGTM! Help text accurately reflects the new defaults.The updated help text clearly communicates the rolling 365-day window behavior to users and is consistent with the implementation in
owasp_create_member_snapshot.py.Also applies to: 47-47
207-209: LGTM! Default date range correctly implements rolling 365-day window.The implementation properly calculates the start date as 365 days before today, ensuring all contributions from the last year are synchronized. This directly addresses issue #3184 and aligns with the PR objective.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/apps/github/common.py (2)
79-83: Consider using theyear_agovariable for consistency.The logic correctly extends the lookback window to 365 days, aligning with the PR objectives. However, for consistency with the issues and pull requests sections (which use the
year_agovariable defined at line 105), consider moving theyear_agodefinition earlier and using it here as well.♻️ Suggested refactor for consistency
Move the
year_agodefinition before the milestones section and use it:if not repository.is_archived: + year_ago = timezone.now() - td(days=365) + # GitHub repository milestones. kwargs = { "direction": "desc", "sort": "updated", "state": "all", } until = ( latest_updated_milestone.updated_at if (latest_updated_milestone := repository.latest_updated_milestone) - else timezone.now() - td(days=365) + else year_ago )Then remove the duplicate definition at line 105.
82-82: Monitor API rate limits and sync performance after deployment.Extending the lookback window from 30 to 365 days will significantly increase the volume of data fetched per sync operation. Consider monitoring:
- GitHub API rate limit consumption: The API has rate limits (typically 5000 requests/hour for authenticated requests). For organizations with many active repositories, this 12x increase in lookback window could approach or exceed these limits.
- Sync duration: Initial syncs and syncs for repositories without prior data will fetch up to 365 days of history, potentially increasing sync times substantially.
- Database growth: Storing a full year of historical data will increase storage requirements.
If rate limits or performance issues emerge, consider:
- Implementing exponential backoff and retry logic for rate limit errors
- Batching repository syncs to spread API calls over time
- Adding progress tracking and resumption capability for long-running syncs
- Monitoring specific high-activity repositories that might consume disproportionate API quota
Also applies to: 116-116, 167-167
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/apps/github/common.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/apps/github/common.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/apps/github/common.py
🔇 Additional comments (3)
backend/apps/github/common.py (3)
105-105: Good variable extraction for reuse.The
year_agovariable efficiently avoids repeating the 365-day calculation for both issues and pull requests sections.
113-117: LGTM!The change correctly extends the issues lookback window to 365 days using the
year_agovariable, aligning with the PR objectives to capture complete yearly activity for contribution heatmaps.
164-168: LGTM!The change correctly extends the pull requests lookback window to 365 days using the
year_agovariable, consistent with the issues section and aligned with the PR objectives.
|
Hi @arkid15r , |
ahmedxgouda
left a comment
ahmedxgouda
left a comment
There was a problem hiding this comment.
Good work. Please address these comments
There was a problem hiding this comment.
I think these updates here are unrelated changes.
There was a problem hiding this comment.
Sure, I will remove the unwanted files
|
Hi @ahmedxgouda , PR is ready for review. |
There was a problem hiding this comment.
It's not syncing the commits, right? Don't we need to write the logic for that?
|
3 participants
Proposed change
This PR update github sync pipeline to fetch all commits, pull requests and issues from the last 365 days across OWASP org.
also updates member snapshot generation to ensure contribution heatmaps and entity stats accurately reflect the full yearly activity window.
Resolves #3184
Checklist
make check-testlocally and all tests passed