Add gitlab repositories information retrieval

[GuillaumeGomez]

Part of #35

This PR refactors how we retrieve repositories information by creating a general Updater running specific ones and add the information retrieval for gitlab repositories.

[pietroalbini]

Did a high level review of the PR. I agree with you that we should generalize, and move to a single database table, a single updater struct and a RepositoryHost trait implementing GitHub or GitLab specific parts.

[GuillaumeGomez]

I made a repositories database which now handles both github and gitlab. Next step will be to clean up a bit the updaters. Having a trait for more generics will be welcome I assume. However, I think it's better to keep both of them separate, it makes going through the code simpler.

[Nemo157]

I think it may make sense to have two levels here, a general RepositoryStatsUpdater that calls into the GithubUpdater/GitlabUpdater (which may be using some shared code). That would simplify the integration points: still only a single thread running the update loop, and only a single call from the builder to have it detect which implementation applies and call into it.

[GuillaumeGomez]

Yes, I have something similar in mind.

[pietroalbini]

I think there is still too much code duplication here. The trait should only contain code specific to the forge in question, so checking if a URL belongs to the forge, loading a single repository by its URL and loading multiple repositories by their IDs. Everything else, including every database interaction, should go into the main updater.

Also, it's probably time to move the whole thing out of utils into src/repositories/{mod,github,gitlab}.rs.

[GuillaumeGomez]

I think there is still too much code duplication here. The trait should only contain code specific to the forge in question, so checking if a URL belongs to the forge, loading a single repository by its URL and loading multiple repositories by their IDs. Everything else, including every database interaction, should go into the main updater.

On it!

Also, it's probably time to move the whole thing out of utils into src/repositories/{mod,github,gitlab}.rs.

Any suggestion for the naming of the new module?

[pietroalbini]

Any suggestion for the naming of the new module?

I guess just repositories.

[GuillaumeGomez]

Moved the repositories files into a repositories folder.

[GuillaumeGomez]

I think it's now ready for review. :)

[GuillaumeGomez]

Fixed conflict.

[GuillaumeGomez]

I also added a test to ensure the regex update is working as expected.

[syphar]

rpa, dockertest (repo is a sub-repo, I forgot how they were called in gitlab)

It works fine once the regex is fixed. :)
I tested it again, it works now.

Also I did another check on some random not found repos, and these were either really wrong, or moved.

phaser (repo url ends with .git, github integration seems to support this)

The repository doesn't exist: https://gitlab.com/bloom42/phaser (maybe it's private?).

Not sure how that failed, I just tried again and it's fine.

with_tempdir, biorustlings (repo was moved, I don't know the how github integration behaves here)

It simply doesn't work I guess? So until they update the repository URL, I don't think there is much we can do about...

let's keep it like that, agreed.

Anyway, I fixed the backward migration and the regex so now things are working as expected. :)

The backwards migration is still broken, I added a fixed sql statement that. (it works in a local test)

[GuillaumeGomez]

The backwards migration is still broken, I added a fixed sql statement that. (it works in a local test)

I just realized I was testing the backward migration with a gitlab crate... Thanks a lot!

[syphar]

so,

from looking at the code I don't see anything else we can test to be sure here. Also I guess the code itself had many iterations with @pietroalbini

Everything I tested works as it should, with gitlab- and github repos

• migrations forward and backwards are fine now
• the crate-detail page is fine with both github and gitlab repos
• backfill and normal update work fine and put in data

of course I might be missing details I don't know about (yet), but IMHO this is good enough to go live. The risk of breaking something major is low, and the revert is easy and tested.

I don't know (yet) how the deploy-process works (order of server-restart and the migrations being executed), depending on that there will be a time where we will have server errors.

Building this in a non-destructive way (without server errors while deploying/migrating) is possible of course, but would add quite some work.

[jyn514]

Could you update the PR description?

I left a lot of comments, but they're mostly nits, no major changes.

[GuillaumeGomez]

Updated the description as well.

[jyn514]

It would be nice to squash the history a bit when you get a chance, things have changed quite a lot since the initial PR. If it's a pain to use logical commits I'm fine with one giant commit.

[GuillaumeGomez]

For the history rewrite, I was waiting for the PR to get approved so I can squash everything in one commit and we're done for. Gonna do that in the next push.