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

[Downloader] More robust repo loading #2121

Merged
merged 4 commits into from
Sep 22, 2018

Conversation

Tobotimus
Copy link
Member

Previously, when downloader was loaded, the RepoManager would spawn a task to load available repos. If one repo failed loading for some reason, the function would raise and the remaining repos would never be loaded, however downloader would still appear to load correctly.

This change handles exceptions better during repo loading, but also, if an unhandled exception is raised, downloader will fail to load as it should.

Also included, as requested in #1968, is the --recurse-submodules flag in cloning/pulling repositories.

This change resolves #1950.

Previously, when downloader was loaded, the RepoManager would spawn a task to load available repos. If one repo failed loading for some reason, the function would raise and the remaining repos would never be loaded, however downloader would still appear to load correctly.

This change handles exceptions better during repo loading, but also, if an unhandled exception is raised, downloader will fail to load as it should.

Also included, as requested in Cog-Creators#1968, is the --recurse-submodules flag in cloning/pulling repositories.

This change resolves Cog-Creators#1950.

Signed-off-by: Toby Harradine <tobyharradine@gmail.com>
Signed-off-by: Toby Harradine <tobyharradine@gmail.com>
Signed-off-by: Toby Harradine <tobyharradine@gmail.com>
Copy link
Contributor

@mikeshardmind mikeshardmind left a comment

Choose a reason for hiding this comment

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

This appears to be fine, and appears to achieve the goals it sets out for without breaking anything else in the process. Might be obsoleted in the work @calebj has been doing for reworking the downloader's internals later on, but it's worth fixing the issues with this now.

@Tobotimus Tobotimus added QA: Passed Used by few QA members. Has been approved by the assigned QA member(s). and removed QA: Needed labels Sep 22, 2018
@Tobotimus Tobotimus merged commit e27682a into Cog-Creators:V3/develop Sep 22, 2018
@Tobotimus Tobotimus deleted the V3/bugfix/1950 branch September 24, 2018 00:37
Tobotimus added a commit to Tobotimus/Red-DiscordBot that referenced this pull request Sep 24, 2018
I somehow managed to botch this in Cog-Creators#2121

Signed-off-by: Toby Harradine <t.harradine@student.unsw.edu.au>
Tobotimus added a commit that referenced this pull request Sep 24, 2018
I somehow managed to botch this in #2121

Signed-off-by: Toby Harradine <t.harradine@student.unsw.edu.au>
@Jackenmen Jackenmen added the Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing. label Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA: Passed Used by few QA members. Has been approved by the assigned QA member(s). Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[V3 Downloader] Missing repos from repo list
4 participants