Skip to content

feat(backend): ingest GitHub issues with idempotent upsert (#74)#90

Closed
Cedarich wants to merge 13 commits intoTheSoftwareDevGuild:mainfrom
Cedarich:feat/backend-github-issues-ingestion-74
Closed

feat(backend): ingest GitHub issues with idempotent upsert (#74)#90
Cedarich wants to merge 13 commits intoTheSoftwareDevGuild:mainfrom
Cedarich:feat/backend-github-issues-ingestion-74

Conversation

@Cedarich
Copy link

@Cedarich Cedarich commented Oct 3, 2025

🚀 PR: Ingest GitHub Issues into Backend Database (#74)

📌 Summary

  • Adds an ingestion path that fetches GitHub issues for specified repositories and persists them with idempotent upsert semantics.
  • Lays the groundwork for reward processing and other downstream features.

🔍 Scope

  • Endpoint: POST /admin/github/sync
  • Accepts payload:
    {
      "repos": ["org/repo", ...],
      "since": "2024-01-01T00:00:00Z" // optional
    }
    
    

close #74

Copy link
Collaborator

@oscarwroche oscarwroche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API interaction needs to be handled in the infrastructure layer instead of the application layer. Please create a service trait in the domain layer and create an implementation in the infrastructure. The application side should have no direct references to infrastructure (Github API, Postgres, etc) concerns

@Cedarich
Copy link
Author

Cedarich commented Oct 5, 2025

@oscarwroche Please review again This refactor enforces clean architecture boundaries: application depends on the GithubApiService trait, while infrastructure provides the HTTP implementation. No direct reqwest or Postgres references remain in the application layer.

@Cedarich Cedarich requested a review from oscarwroche October 5, 2025 09:55
Copy link
Collaborator

@oscarwroche oscarwroche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Cedarich Looks great overall! Just a comment about removing the query! macro in the indexer

@Cedarich
Copy link
Author

Cedarich commented Oct 5, 2025

@oscarwroche

  • I restored compile-time checking in the indexer by switching the list query to sqlx::query! and kept the event_type JSON handling intact.
  • The offline cache for the SELECT is already generated and committed at indexer/.sqlx/query-c8d8e35…json .
  • The insert_many path is temporarily using sqlx::query (dynamic) to unblock diagnostics while my local database is unavailable. This means compile-time validation for the insert is pending.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't know why there are changes to the indexer files, they don't have anything to do with the goal of this PR. Creating this migration is unnecessary

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review 🙏

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is the migration renamed ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I still don't understand why there are any changes to the indexer files

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is the migration renamed ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I still don't understand why there are any changes to the indexer files

@joelamouche
Copy link
Contributor

@Cedarich what's the update on this pls?

@joelamouche
Copy link
Contributor

close due to inactivity

@joelamouche joelamouche closed this Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ingest GitHub Issues into Backend Database

4 participants

Comments