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] Actually use disabled key in updates #3173

Closed
mikeshardmind opened this issue Dec 7, 2019 · 4 comments · Fixed by #3203
Closed

[Downloader] Actually use disabled key in updates #3173

mikeshardmind opened this issue Dec 7, 2019 · 4 comments · Fixed by #3203
Assignees
Labels
Type: Bug

Comments

@mikeshardmind
Copy link
Contributor

@mikeshardmind mikeshardmind commented Dec 7, 2019

Regression from 3.1.8 -> dev, disabled key doesnt prevent install False Alarm on regression.

Same from 3.1.8 -> dev, disabled key doesnt prevent loading an updated cog which was made disabled.

CC: @jack1142 , discord API issues may make it easier to discuss issue here.

@mikeshardmind mikeshardmind added the Type: Bug label Dec 7, 2019
@jack1142
Copy link
Member

@jack1142 jack1142 commented Dec 7, 2019

Due to current discord's issues I haven't actually been able to test any of this, but after taking a second look at the code, it looks like the issue with [p]cog install* is probably me overlooking something - those commands are most likely preventing install of a disabled cog in both 3.1.8 and dev.

But the issue with [p]cog update is still valid, at least to some degree.

@mikeshardmind
Copy link
Contributor Author

@mikeshardmind mikeshardmind commented Dec 7, 2019

I'm gonna go ahead and assign you on this one, but there's no rush on this. It will be nice to have this working for 3.2 and we have a lot to still handle for 3.2 with the only real timeframe for release being "when it's ready"

If the scope of the issue is wrong, it can be corrected after verification, I just went with what you presented to get us started here.

@jack1142
Copy link
Member

@jack1142 jack1142 commented Dec 7, 2019

I was more worried about the possible regression here, but from the code it looks like [p]cog install* commands most likely work fine (waiting to be able to test it 😄).
From what I've been able to figure out from code, [p]cog update* shouldn't get any errors because of the cog being disabled so there should be no regression here either.

The only issue (but at least not regression) should be cogs showing as possible to update when the version they can be updated to is disabled. Therefore [p]cog checkforupdates will show incorrect output and [p]cog update* commands will update that cog to disabled version.

But of course, all this can be corrected in issue description after I'll verify it.

@jack1142
Copy link
Member

@jack1142 jack1142 commented Dec 8, 2019

After testing, looks like [p]cog install* commands are fine (no regression, no issues related to disabled cogs).

Only [p]cog checkforupdates and [p]cog update* commands are not handling disabled cogs correctly - cogs can be updated despite the updated version having a disabled key set to true. Something I didn't expect though - this is a a regression from 3.1.8 as it turns out that it's not possible to update to disabled version of a cog on 3.1.8.

@mikeshardmind can you update issue description accordingly? Thanks.

@mikeshardmind mikeshardmind changed the title [Downloader] actually use disabled key [Downloader] Actually use disabled key in updates Dec 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants