search

src-d / ghsync

GitHub API v3 > PostgreSQL
https://sourced.tech
GNU General Public License v3.0
9 stars 8 forks source link

List endpoints with repository transaction #34

Open carlosms opened 5 years ago

carlosms commented 5 years ago

Related to #30. Done on top of #32, only the last commit is relevant 5249912.

Since the issues, PRs and the repository are all committed at the same time, we can remove the FindOne checks for issues and PRs because we already know the repository was not found.

After testing this change the download times for src-d did not change, it's still around 15m. That's why I'm submitting this as a separate PR, in case we consider this complicates the code and prefer to just merge #32.

smacker commented 5 years ago

If it doesn't improve performance I don't think we should merge it. The problem not really with the code but in changing something without proper consideration of tradeoffs and effect on future development. In my opinion, after the release, we need to come back to a bigger picture with support for sync not only import, importing all entities not only PR/Issues. And it might affect the way we work with DB. If we find this approach is actually correct we can always rebase. There shouldn't be many conflicts.

smacker commented 5 years ago

One of my concerns is if we remove FindOne we won't be able to do re-sync without dropping everything and re-importing completely from scratch. Let's move it back to TODO while we don't have DD/Roadmap for ghsync.