Condition the OSS code to migrate the changes to ent #1146
Conversation
Juanadelacuesta
force-pushed
the
NOJIRA-ent-updates
branch
from
1f0b67e to
f316c89
Compare
Juanadelacuesta
force-pushed
the
NOJIRA-ent-updates
branch
from
f316c89 to
d01f41e
Compare
Juanadelacuesta
force-pushed
the
NOJIRA-ent-updates
branch
from
10de3f0 to
e96c2a6
Compare
…idation for nil policy
Juanadelacuesta
force-pushed
the
NOJIRA-ent-updates
branch
from
b0df528 to
28d22f7
Compare
Juanadelacuesta
force-pushed
the
NOJIRA-ent-updates
branch
from
760aae8 to
8765b8f
Compare
Juanadelacuesta
force-pushed
the
NOJIRA-ent-updates
branch
from
256e733 to
158d587
Compare
… test and account for vertical scaling where direction is none
Juanadelacuesta
force-pushed
the
NOJIRA-ent-updates
branch
from
46afada to
5b0e999
Compare
policy/handler_test.go
Outdated
| must.Eq(t, tt.wantAction.Count, result.action.Count) | ||
| must.NotNil(t, result.handler) | ||
| must.Eq(t, tt.wantHandler.check.Name, result.handler.check.Name) | ||
| //must.Eq(t, tt.wantHandler.check.Name, result.handler.check.Name) |
There was a problem hiding this comment.
| //must.Eq(t, tt.wantHandler.check.Name, result.handler.check.Name) |
sdk/policy.go
Outdated
| func (p *ScalingPolicy) Validate() error { | ||
| if p == nil { | ||
| return nil | ||
| return errors.New("empty policy") |
There was a problem hiding this comment.
Just double-checking, but this looks like a behavior change? The relationship between sdk.ScalingPolicy and scaling.policy block has been a little fuzzy for me... but the default is for it to be nil, isn't it?
There was a problem hiding this comment.
you are right, let's revert this one
| for source, handlers := range m.handlers { | ||
| for id := range handlers { | ||
| m.stopHandler(source, id) | ||
| delete(handlers, id) |
There was a problem hiding this comment.
This looks like a behavior change as well?
There was a problem hiding this comment.
I realised it was missing, I will fix the comments
| delete(handlers, id) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This doesn't include delete(m.handlers, source) call that stopHandlers does. Should we remove the source if len(handlers) == 0 after stopping by type?
There was a problem hiding this comment.
This function is meant to replace the one that would stop all the vertical workers that where in charge of the recommendations in case of an invalid license, Im not usre removing the source is needed here.
| h.log.Debug("check selected", "name", winner.handler.check.Name, | ||
| "direction", winner.action.Direction, "count", winner.action.Count) | ||
| h.log.Debug("check selected", "direction", winner.action.Direction, | ||
| "count", winner.action.Count) |
There was a problem hiding this comment.
I dont remember, I think it was too long and made the logs too verbose
…heck for nil policy
In order to migrate the policy worker redesign into the ent version, some parts of the code needed to be rearranged. This PR does it, plus addressing a missing deletion clean up when stoping the handlers.