-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Ephemeral Nodes for via Session behavior settings. #487
Conversation
Added a "delete" behavior for session invalidation, in addition to the default "release" behavior. On session invalidation, the sessions Behavior field is checked and if it is set to "delete", all nodes owned by the session are deleted. If it is "release", then just the locks are released as default.
@@ -28,6 +28,9 @@ func (s *Session) Apply(args *structs.SessionRequest, reply *string) error { | |||
if args.Session.Node == "" && args.Op == structs.SessionCreate { | |||
return fmt.Errorf("Must provide Node") | |||
} | |||
if args.Session.Behavior == "" { |
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.
Let's be more conservative. Better to switch on Behavior
. If blank, default to Release, if Release or Delete great, if default (e.g. undefined behavior) return an error.
Looking really good! I left some minor comments! |
…cified, unrecognized
@@ -28,6 +28,13 @@ func (s *Session) Apply(args *structs.SessionRequest, reply *string) error { | |||
if args.Session.Node == "" && args.Op == structs.SessionCreate { | |||
return fmt.Errorf("Must provide Node") | |||
} | |||
switch args.Session.Behavior { |
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 think there should be a case for the empty string which does default, and return an error in the default case. e.g. behavior = foobar should return an error.
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.
OK, agreed. Will fix this and the other file as well.
On Nov 20, 2014 5:58 PM, Armon Dadgar notifications@github.com wrote:
In consul/session_endpoint.go:
@@ -28,6 +28,13 @@ func (s *Session) Apply(args *structs.SessionRequest, reply *string) error {
if args.Session.Node == "" && args.Op == structs.SessionCreate {
return fmt.Errorf("Must provide Node")
}
switch args.Session.Behavior {
I think there should be a case for the empty string which does default, and return an error in the default case. e.g. behavior = foobar should return an error.
—
Reply to this email directly or view it on GitHubhttps://github.com//pull/487/files#r20684732.
Looking great, I think just two more small changes |
… unrecognized values
Great! |
Ephemeral Nodes for via Session behavior settings.
It will be in the next release, likely 0.5.0 |
Nothing firm, likely in January. |
Added a "delete" behavior for session invalidation, in addition to
the default "release" behavior. On session invalidation, the sessions
Behavior field is checked and if it is set to "delete", all nodes owned
by the session are deleted. If it is "release", then just the locks
are released as default.
Crack at implementing: #238