Bug 1504729 - Log job state when getting last op #505
Conversation
openshift-ci-robot
added
the
size/XS
| a.log.Debug(fmt.Sprintf("service_id: %s", req.ServiceID)) // optional | ||
| a.log.Debug(fmt.Sprintf("plan_id: %s", req.PlanID)) // optional | ||
| a.log.Debug(fmt.Sprintf("operation: %s", req.Operation)) // this is provided with the provision. task id from the work_engine | ||
| a.log.Debugf("service_id: %s", req.ServiceID) |
There was a problem hiding this comment.
Thanks for killing the fmt.Sprint, there's a lot of that left over from the old days :)
pkg/broker/broker.go
Outdated
| a.log.Debug(fmt.Sprintf("operation: %s", req.Operation)) // this is provided with the provision. task id from the work_engine | ||
| a.log.Debugf("service_id: %s", req.ServiceID) | ||
| a.log.Debugf("plan_id: %s", req.PlanID) | ||
| a.log.Debugf("operation: %s", req.Operation) // this is provided with the provision. task id from the work_engine |
There was a problem hiding this comment.
Nit: the comment is a little inaccurate, it'll be there with every async op, not just provision.
pkg/broker/broker.go
Outdated
| @@ -1004,7 +1004,7 @@ func (a AnsibleBroker) LastOperation(instanceUUID uuid.UUID, req *LastOperationR | |||
| */ | |||
| a.log.Debugf("service_id: %s", req.ServiceID) | |||
| a.log.Debugf("plan_id: %s", req.PlanID) | |||
| a.log.Debugf("operation: %s", req.Operation) // this is provided with the provision. task id from the work_engine | |||
There was a problem hiding this comment.
Why is this an erroneous comment? The operation is expected to be the job token (task_id) from the work_engine that happened during the provision. At a minimum I'd remove provision and make it clear that we expect operation to be the job token id.
There was a problem hiding this comment.
I just remarked it wasn't entirely accurate since all async jobs will set that to be the job token, not just provision.
djzager
force-pushed
the
instance-state-logging
branch
2 times, most recently
from
bcb5fe2
to
2a6fdda
Compare
|
If this was changing something in docs or build file stuff I would let it slide. This is actually changing mainline broker code so a bugzilla is required. |
djzager
force-pushed
the
instance-state-logging
branch
from
2a6fdda
to
a10ae7d
Compare
djzager
changed the title
Simple enhancement so that we can see the state of the last operation when the service-catalog requests it.