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

Add logger to cache entry to catch exception #37292

Merged

Conversation

lindexi
Copy link
Member

@lindexi lindexi commented Jun 2, 2020

As we can see, we can not know the EvictionCallback invoked failed. I want to add the logger to CacheEntry to log the exception

But I don't know if it is suitable to use LogInformation to log the exception

Thank you

@maryamariyan
Copy link
Member

@lindexi there is no issue tracking this is there?

@ghost
Copy link

ghost commented Jul 16, 2020

Tagging subscribers to this area: @eerhardt
Notify danmosemsft if you want to be subscribed.

Copy link
Member

@maryamariyan maryamariyan left a comment

Choose a reason for hiding this comment

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

LGTM, this could work. I retriggered CI to see what failures there are if any.

Copy link
Member

@maryamariyan maryamariyan left a comment

Choose a reason for hiding this comment

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

missing namespace on CacheEntry for ILogger is giving compile error

@lindexi
Copy link
Member Author

lindexi commented Jul 17, 2020

@maryamariyan Thank you and I do not know which the EventId is right to use.

@maryamariyan
Copy link
Member

close and reopening to retrigger the CI

@lindexi
Copy link
Member Author

lindexi commented Aug 5, 2020

@maryamariyan The CI is failed, and what should I do?

@maryamariyan
Copy link
Member

The PR fails on the step to build native dependencies. @lindexi maybe because the PR was stale a little bit. Do you mind doing a rebase to update the PR?

@lindexi lindexi force-pushed the t/lindexi/set-logger-to-cache-entry branch from a312baa to 4d99c9a Compare August 6, 2020 00:34
@maryamariyan maryamariyan merged commit 7c9db60 into dotnet:master Aug 11, 2020
@lindexi lindexi deleted the t/lindexi/set-logger-to-cache-entry branch August 12, 2020 00:38
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants