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

Added 'RecordExists' flag to perisistent store so that grains can det… #6580

Merged
merged 3 commits into from Jul 7, 2020
Merged

Added 'RecordExists' flag to perisistent store so that grains can det… #6580

merged 3 commits into from Jul 7, 2020

Conversation

zeus82
Copy link
Contributor

@zeus82 zeus82 commented Jun 5, 2020

…ect if a record exists already

@zeus82
Copy link
Contributor Author

zeus82 commented Jun 5, 2020

This PR is for issue 6517. I just want to make sure the approach I took is acceptable. If it is I will add some unit tests.

@EdeMeijer
Copy link
Contributor

Good idea, but how will this affect my custom written storage providers? Just adding a method to an existing interface like that is a backwards incompatible change I'm afraid.

@zeus82
Copy link
Contributor Author

zeus82 commented Jun 23, 2020

I didn't change IGrainStorage, so this will not break custom storage providers. Until the custom provider is upgraded, that value will always be false.

@benjaminpetit
Copy link
Member

Thanks for your PR (and sorry for the delay)! I think your approach is totally fine, it would be great if you could add some unit tests. If you don't have time, I can add them myself to your PR.

Let me know what you prefer.

@zeus82
Copy link
Contributor Author

zeus82 commented Jul 6, 2020

Thanks @benjaminpetit . I added some asserts to some of the tests, I think that covers it, but if I missed anything, please let me know!

@benjaminpetit
Copy link
Member

/azp run Azure DevOps - Functional

LGTM, I will merge it as soon as functional pass. Failure isn't related, but you never know...

Many thanks

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@benjaminpetit benjaminpetit merged commit fe87e13 into dotnet:master Jul 7, 2020
sergeybykov pushed a commit that referenced this pull request Jul 8, 2020
@zeus82 @sergeybykov
Added 'RecordExists' flag to perisistent store so that grains can det… (
8e6b499 
#6580)

(cherry picked from commit fe87e13)
sergeybykov pushed a commit to sergeybykov/orleans that referenced this pull request Aug 13, 2020
@zeus82 @sergeybykov
Added 'RecordExists' flag to perisistent store so that grains can det… (
1559ad5 
dotnet#6580)

* Added 'RecordExists' flag to perisistent store so that grains can detect if a record exists already

* Updates some test method to check RecordExists flag

Co-authored-by: smanickam <smanickam@metocean.com>

(cherry picked from commit fe87e13)
@sergeybykov sergeybykov mentioned this pull request Aug 14, 2020
Merged
ReubenBond pushed a commit that referenced this pull request Aug 14, 2020
@zeus82 @ReubenBond
Added 'RecordExists' flag to perisistent store so that grains can det… (
ac7d532 
#6580)

* Added 'RecordExists' flag to perisistent store so that grains can detect if a record exists already

* Updates some test method to check RecordExists flag

Co-authored-by: smanickam <smanickam@metocean.com>

(cherry picked from commit fe87e13)
@pentp pentp mentioned this pull request Aug 24, 2020
Closed
@seniorquico seniorquico mentioned this pull request Aug 27, 2020
Closed
@ygrinevich ygrinevich mentioned this pull request Sep 17, 2020
Closed
@ReubenBond ReubenBond mentioned this pull request Jun 9, 2022
Open
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@benjaminpetit benjaminpetit

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@zeus82 @EdeMeijer @benjaminpetit