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

Refetch local metadata cache if corruption is detected #31509

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Jan 14, 2025

Addresses one of the points in #31496.

Not going to lie, this is mostly best-effort stuff (while the refetch is happening, metadata lookups using the local source will fail), but I see this as a marginal scenario anyways.

Addresses one of the points in ppy#31496.

Not going to lie, this is mostly best-effort stuff (while the refetch is
happening, metadata lookups using the local source *will* fail), but I
see this as a marginal scenario anyways.
@bdach bdach added the area:import dealing with importing of data from stable or file formats label Jan 14, 2025
@smoogipoo smoogipoo requested a review from peppy January 14, 2025 11:19
@pull-request-size pull-request-size bot added size/M and removed size/S labels Jan 22, 2025
peppy
peppy previously approved these changes Jan 22, 2025
@peppy
Copy link
Member

peppy commented Jan 22, 2025

Seems to work well. I've applied some style changes, please check you're okay with the changes @bdach .

Comment on lines 126 to 127
if (cacheDownloadRequest != null)
throw;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This looks like a behavioural change? I think this should return false? If the intention was to rethrow to the catch (Exception ex) block below then I'm not sure that's how this works.

In an isolated console project, when I do something like

try
{
   throw new InvalidOperationException();
}
catch (InvalidOperationException)
{
   throw;
}
catch
{
   Console.WriteLine("caught");
}

then the resulting program is not printing to the console, it's throwing the error:

Unhandled exception. System.InvalidOperationException: Operation is not valid due to the current state of the object.
   at Program.<Main>$(String[] args) in /home/dachb/Documents/Projekty/ConsoleApp1/ConsoleApp1/Program.cs:line 5

Copy link
Member

Choose a reason for hiding this comment

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

right, that's unfortunate. i thought that's how it worked but i guess not.

Copy link
Member

Choose a reason for hiding this comment

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

I made one more attempt.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Seems better now yeah.

@peppy peppy self-requested a review January 22, 2025 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:import dealing with importing of data from stable or file formats size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants