Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Container Rehydration from a snapshot taken from detached container. #3061
Container Rehydration from a snapshot taken from detached container. #3061
Changes from 34 commits
59f1e16
4bffb77
5573ffa
59d66b5
d240a3a
f99f68e
217aacc
1bb9da0
6fcce12
3f903b7
e255f54
59a4ff2
737111d
cfde78c
0a38dff
acc535a
918a5c4
84bc42e
808d530
7150af4
60f977f
4bd5d7e
f93b1df
79592b1
2876424
acd860d
e52d2ac
3f46d22
a19116f
eae3ec6
562b29b
55d76e2
d18fd5f
0525797
7c3e821
c395773
cf97acd
a1a07a8
545f304
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's easy to differentiate interfaces, I'd rather have single IFluidCodeDetails | ISnapshotTree argument.
Or a type that differentiates them:
{ create: true, code: IFluidCodeDetails } | { create: false, snapshot: ISnapshotTree}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chose this one: { create: true, code: IFluidCodeDetails } | { create: false, snapshot: ISnapshotTree}. It is more explicit, infact it was like this before but later moved to other one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't like like this type. i'd rather just have separate methods. also, can the code proposal be updated on a detached container? i think we will want to support this if we don't
In reply to: 479472885 [](ancestors = 479472885)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you basically already have two methods anyway. i don't see a whole lot of value in coupling these
In reply to: 483120067 [](ancestors = 483120067,479472885)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 2 methods one of which load from codeDetails and the other which load from snapshot are private methods. Loader calls create method on container which is public if it wants to create a detached container either from snapshot or code details. So this type is just used here. Aesthetically this looks good to me:
{ create: true, code: IFluidCodeDetails } | { create: false, snapshot: ISnapshotTree}
Code can be proposed again by the quorum, isn't it? I mean can we update the code and load from snapshot at the same time? I don't think so.