Skip to content

Fix/GitHub issues sync improvements#1843

Closed
Selectus2 wants to merge 7 commits intocodetriage:mainfrom
Selectus2:fix/github-issues-sync-improvements
Closed

Fix/GitHub issues sync improvements#1843
Selectus2 wants to merge 7 commits intocodetriage:mainfrom
Selectus2:fix/github-issues-sync-improvements

Conversation

@Selectus2
Copy link

Description

This PR fixes several issues related to GitHub issues synchronization and Ruby 3.2 compatibility:

Ruby 3.2 Compatibility:

  • Added explicit require "logger" in config/boot.rb and config/application.rb (Ruby 3.2 no longer auto-loads Logger)
  • Pinned minitest to ~> 5.0 (version 6.0+ removed minitest/mock)

Asset Pipeline Fix:

  • Fixed local-time JavaScript asset path for local_time gem 3.x (local-time.es2017-umd)

GitHub Issues Sync Improvements:

  • Fixed last_page? pagination handling when last_url is nil (was causing URI::InvalidURIError)
  • Added per_page=100 parameter to fetch more issues per API request (reduced API calls)
  • Fixed issues_count to exclude Pull Requests (GitHub's Issues API returns both issues and PRs)
  • Added smart rate limiting using rate_limit_sleep! to avoid hitting GitHub API limits
  • Made per_page configurable via GITHUB_ISSUES_PER_PAGE env variable

Code Clarity:

  • Added is_pull_request?, is_issue? methods and pull_requests, issues_only scopes to Issue model
  • Added detailed comments explaining GitHub API behavior (PR vs Issue distinction)

Test Fixes:

  • Updated VCR configuration with custom matcher to ignore per_page parameter
  • Fixed test stubs for Ruby 3.2 keyword argument changes
  • Updated test expectations for new last_page? behavior

Related Issue

Fixes #1841

Motivation and Context

  1. Ruby 3.2 compatibility - The app failed to start on Ruby 3.2 due to changes in how Logger and minitest are loaded
  2. Incorrect issues count - The issues_count was including Pull Requests, showing 169 instead of 136 actual issues
  3. Pagination failures - The PopulateIssuesJob was failing with URI::InvalidURIError when GitHub didn't return a last_url in pagination headers
  4. API efficiency - Default per_page=30 required more API calls; now uses per_page=100 (GitHub's maximum)
  5. Rate limiting - Added protection against GitHub API rate limits during bulk operations

How Has This Been Tested?

  • Ran full test suite: bin/rake test - 211 runs, 403 assertions, 0 failures, 0 errors
    • Manually tested PopulateIssuesJob against real GitHub repos (aasm/aasm)
    • Verified issues count matches GitHub UI (136 issues, excluding PRs)
    • Tested with different GITHUB_ISSUES_PER_PAGE values to ensure configurability

Screenshots (if appropriate):

Screenshot 2026-01-21 at 6 52 34 PM

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
    • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
    • I have read the CONTRIBUTING document.
    • I have added tests to cover my changes.
    • All new and existing tests passed.

@Selectus2
Copy link
Author

@schneems it would be really great if you get chance to check this as we are planning to kickstart open source Friday event at our organization and codetriage is base thing for us to let people find the issues

@Selectus2
Copy link
Author

@prathamesh-sonpatki do review this if you get a chance to review

@schneems
Copy link
Member

schneems commented Feb 3, 2026

Thanks for the PR.

I didn't merge this as it had a lot of intertwined and (seemingly) unrelated changes and it was unclear why the fix was "the fix" i.e. the problem wasn't spelled out as to what would make you think that's the problem in such a way that I could validate that you've fixed it correctly (or alternatively help provide more information to you).

#1841 is caused by hitting limits of the database as many tables are using "integer" and "serial" instead of "bigint" and "bigserial" (64bit integers). And by a failure in git_hub_bub when github fails to send us the last_url (which is seemingly new behavior from them).

Database failure

E, [2026-02-02T15:37:35.582127 #5] ERROR -- : [ActiveJob] [PopulateIssuesJob] [cffcb87a-a234-487b-9bf8-07fefe0c8213] Error performing PopulateIssuesJob (Job ID: cffcb87a-a234-487b-9bf8-07fefe0c8213) from Sidekiq(default) in 550.91ms: ActiveRecord::RangeError (PG::NumericValueOutOfRange: ERROR:  integer out of range
):
/app/vendor/bundle/ruby/3.2.0/gems/rack-mini-profiler-3.1.1/lib/patches/db/pg.rb:69:in `exec_params'
/app/vendor/bundle/ruby/3.2.0/gems/rack-mini-profiler-3.1.1/lib/patches/db/pg.rb:69:in `exec_params'
# ...
irb(main):008:0> IssueAssignment.count
D, [2026-02-02T15:50:41.899327 #5] DEBUG -- :   IssueAssignment Count (19738.7ms)  SELECT COUNT(*) FROM "issue_assignments"
=> 24965798

There's not an easy way you could have guessed or known that without access to the database, so I don't fault you for not fixing, but it did take time to review the fix and determine that I didn't have enough information and needed to explore the problem myself.

Last Url failure

The git_hub_bub failure, you did find and fix:

      # If no next_url, we're on the last page
      return true if response.next_url.nil?
      # If next_url exists but last_url is nil, there are more pages
      # (git_hub_bub gem bug - it tries to parse nil last_url)
      return false if response.last_url.nil?

But it wasn't clear that was the fix, or why. It's best served by fixing it "upstream" in the gem (which I also own) but this fix is okay too for a short term change. In the future, it would have been really helpful if the PR had just that change coupled with some hard output or evidence (reproducible) on how to validate what the bug was and that it was correctly fixed.

I appreciate the pings. I missed the other issue come in and didn't catch that ALL issues new had stopped populating. So your PR was still helpful in raising awareness of a more serious issue.

@schneems schneems closed this Feb 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

repo rsyslog/rsyslog always shows 0 issues - re-init possible?

2 participants