Skip to content
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

feat: Dynamic repo registration #351

Merged
merged 48 commits into from
Feb 17, 2025
Merged

Conversation

egekocabas
Copy link
Member

@egekocabas egekocabas commented Feb 7, 2025

Warning

❗ ❗ ❗ This PR contains a migration script ❗ ❗ ❗

Motivation

Enable dynamic repository syncing by accurately tracking which repositories have the GitHub App installed. This ensures our system reflects real-time changes when repositories are added or removed.

Description

  • Webhook Listener Refactoring
    • Updated GitHubCustomMessageHandler to deserialize events and delegate processing to extenders, following the org.kohsuke:github-api library implementation that we have.
    • Introduced isGlobalEvent() in both GitHubCustomMessageHandler and GitHubMessageHandler to differentiate global events (like installation_repositories) from repository-specific ones.
  • Subscription to global events
    • Modified NatsConsumerService.getSubjects to correctly subscribe to global events using the pattern github.?.?.installation_repositories.
  • Repository Sync Foundation
    • Updated GitHubFacade to fetch the list of repositories where the GitHub App is installed.
      • If the auth type is PAT, then it returns repositories defined in the environment variable.
  • GitHubDataSyncService Refactoring
    • Refactored the service to sync individual repositories using the repository name with owner.
    • Added repositoryNameWithOwner and status columns to DataSyncStatus to maintain dedicated sync records.
    • Did not added cascade deletion for DataSyncStatus since the repository record isn’t available during the initial sync. Also it is not that important for DataSyncStatus.
    • Didn't make it @Async since it was causing so much errors related with User table. Further refactoring and making 'data syncing' async can be achieved if we add repository_id column to User table.
  • SyncService Class Updates
    • Moved all the methods except the process methods to a new class called GitHubDataSyncOrchestrator. This change was necessary since we used to call these individual process methods inside it's own bean (SyncService) but this was causing self-invocation for the @Transactional annotation.
  • GitHubInstallationRepositoriesMessageHandler
    • Event listener for GitHub App installed / removed to a repository events.
    • Whenever a repository removed the GitHub App, then we remove its data from db.
    • Whenever a repository adds the GitHub App, then we start data sync for that repository.
    • At the end of this event handler, we update the subjects we listen from NATS.

Copy link

codacy-production bot commented Feb 7, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for 088d91f1 0.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (088d91f) Report Missing Report Missing Report Missing
Head commit (f68172c) 4844 326 6.73%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#351) 517 0 0.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Footnotes

  1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

@github-actions github-actions bot added size:XL and removed size:L labels Feb 7, 2025
@github-actions github-actions bot added size:XXL and removed size:XL labels Feb 8, 2025
@helios-aet helios-aet bot deployed to test-server-3 February 8, 2025 22:37 Active
@egekocabas egekocabas marked this pull request as ready for review February 16, 2025 19:40
@egekocabas egekocabas requested a review from a team as a code owner February 16, 2025 19:40
Copy link
Contributor

@TurkerKoc TurkerKoc left a comment

Choose a reason for hiding this comment

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

Thanks for this important update! Now it's extremely easy for anyone in ls1intum to install our Github App and start using Helios!

We did a testings session with @egekocabas and tried locally installing/removing our dummy Github App into our dummy repo. It works like a charm 🚀

Besides some minor issues we've talked, code also looks fantastic. Let's address them.

Also it would be great if we can add some documentation to guide repository owners to setup their repo in Helios. Here is a discussion for that.
Please also do not forget to mention how they should setup their deployment workflow!

Thanks for the efforts!

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Feb 17, 2025
Copy link
Contributor

@TurkerKoc TurkerKoc left a comment

Choose a reason for hiding this comment

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

Great Job! Thanks for addressing al the points we've discussed 🚀

@TurkerKoc TurkerKoc merged commit d85c591 into staging Feb 17, 2025
15 checks passed
@TurkerKoc TurkerKoc deleted the feat/dynamic-repo-registration branch February 17, 2025 01:33
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.

Required steps to setup new repo in Helios
2 participants