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.

Sign up for GitHub

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

Jump to bottom

Extend MemoryCacheOptions with TrackLinkedCacheEntries #57648

Merged
merged 6 commits into from
Sep 14, 2021
Merged

Extend MemoryCacheOptions with TrackLinkedCacheEntries #57648

merged 6 commits into from
Sep 14, 2021

Conversation

adamsitnik
Copy link
Member

fixes #45592

@adamsitnik adamsitnik added area-Extensions-Caching needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet labels Aug 18, 2021
@adamsitnik adamsitnik added this to the 7.0.0 milestone Aug 18, 2021
@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Aug 18, 2021

Tagging subscribers to this area: @eerhardt, @maryamariyan, @michaelgsharp
See info in area-owners.md if you want to be subscribed.

Issue Details

fixes #45592

Author: adamsitnik
Assignees: -
Labels:

area-Extensions-Caching, needs-breaking-change-doc-created
Milestone: 7.0.0

@runfoapp runfoapp bot mentioned this pull request Aug 18, 2021
Closed
@adamsitnik @stephentoub
Apply suggestions from code review
4684c51 
Co-authored-by: Stephen Toub <stoub@microsoft.com>
@eerhardt
Copy link
Member 

    public async Task OnceExpiredIsSetToTrueItRemainsTrue()

Any reason why this test wasn't turned into a Theory?

Refers to: src/libraries/Microsoft.Extensions.Caching.Memory/tests/CacheEntryScopeExpirationTests.cs:513 in 4684c51. [](commit_id = 4684c51, deletion_comment = False)

adamsitnik and others added 2 commits August 20, 2021 06:07
@adamsitnik @eerhardt
Apply suggestions from code review
8b90f37 
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
@adamsitnik
update comment
a8bb4ab
@adamsitnik
Copy link
Member Author

adamsitnik commented Aug 20, 2021 •
edited
Loading

Any reason why this test wasn't turned into a Theory?

There is just one entry being created in this test (no dependencies to other linked entries) and it just verifies whether setting entry as expired is thread-safe (not related to tracking)

eerhardt
eerhardt approved these changes Aug 23, 2021
View reviewed changes
Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@eerhardt
Copy link
Member

Failures are unrelated. Timing out due to dotnet/core-eng#14346

@eerhardt eerhardt merged commit f2ce707 into dotnet:main Sep 14, 2021
@EgorBo
Copy link
Member

EgorBo commented Sep 23, 2021

arm64 improvements: dotnet/perf-autofiling-issues#1537

@adamsitnik adamsitnik deleted the issue45592 branch October 15, 2021 14:27
@adamsitnik
Copy link
Member Author

To be honest I've not merged it because I wanted to check if something in ASP.NET repo is going to get broken with this change. It has been a month since it was merged and nobody started screaming. I guess it just proves that it was good idea to introduce the breaking change and disable the tracking by default.

@BrennanConroy BrennanConroy mentioned this pull request Oct 30, 2021
Closed
@sebastienros
Copy link
Member

@adamsitnik someone finally screamed

@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2021
@ericstj
Copy link
Member

ericstj commented Oct 4, 2022

@adamsitnik were you planning to create a breaking change document for this?

@ericstj ericstj assigned adamsitnik and unassigned eerhardt Oct 4, 2022
@maryamariyan
Copy link
Member

I just added dotnet/docs#31739

@maryamariyan maryamariyan removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 12, 2022
@ericstj ericstj added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label Oct 12, 2022
@ghost ghost added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 12, 2022
@ericstj ericstj removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 12, 2022
@adamsitnik
Copy link
Member Author

I just added dotnet/docs#31739

@maryamariyan big thanks for doing that!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stephentoub stephentoub stephentoub left review comments

@eerhardt eerhardt eerhardt approved these changes

@maryamariyan maryamariyan Awaiting requested review from maryamariyan

Assignees

@adamsitnik adamsitnik

Labels
area-Extensions-Caching breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. new-api-needs-documentation
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.

[MemoryCache] Add possibility to disable linked cache entries
7 participants
@adamsitnik @eerhardt @EgorBo @sebastienros @ericstj @maryamariyan @stephentoub