[sdk#999] Make sandbox entries restartable #1002
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bolodya1997
force-pushed
the
sdk#999/sandbox-restartable
branch
from
63c3a1b to
fa7af68
Compare
Bolodya1997
requested review from
denis-tingaikin,
d-uzlov,
glazychev-art and
Mixaster995
glazychev-art
approved these changes
Jun 30, 2021
Mixaster995
approved these changes
Jun 30, 2021
denis-tingaikin
requested changes
Jun 30, 2021
pkg/tools/grpcutils/url.go
Outdated
| @@ -102,3 +102,15 @@ func CheckURLFree(u *url.URL) bool { | |||
| } | |||
| return err == nil | |||
| } | |||
|
|
|||
| // CloneURL clones given url | |||
There was a problem hiding this comment.
- I'm not sure that CloneURL should be located in grpcutils.
- Can we use struts instead pointers to get rid of CloneURL?
There was a problem hiding this comment.
- I'm not sure that CloneURL should be located in grpcutils.
I agree, both CloneURL and CheckURLFree are only used in tests, but:
- I am afraid of creating some general
testutilspackage, because it can urgently turn into some very dirty package with lots of different methods. - We probably can add
urlutilspackage for these, but I am not sure that it would be better than storing them ingrpcutils.
Any thoughts on this?
- Can we use struts instead pointers to get rid of CloneURL?
Here we need to refactor nsmgr, client, registryclient, connect packages to use URL struct as a connectTo - it would change API and so affect all deployments without any actual need for them (because it is a test specific case).
We cannot make it on the sandbox side, because in all cases this connectTo is passed via the non-sandbox options.
So for me it still looks like that using CloneURL in tests is the better solution.
Thoughts?
added 4 commits
July 12, 2021 09:41
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Bolodya1997
force-pushed
the
sdk#999/sandbox-restartable
branch
from
e0ffc1a to
59adaf8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Makes sandbox entries cancelable and restartable:
RegistryEntryNSMgrEntryEndpointEntryIssue link
#999
How Has This Been Tested?
Types of changes