Feature/6238: Keeper runs #4210
Conversation
|
I see that you haven't updated any CHANGELOG files. Would it make sense to do so? |
connorwstein
requested review from
j16r,
RyanRHall,
samsondav,
se3000 and
spooktheducks
as code owners
connorwstein
force-pushed
the
feature/6238-keeper-runs
branch
from
978b0f5
to
40b0a3c
Compare
connorwstein
force-pushed
the
feature/6238-keeper-runs
branch
from
40b0a3c
to
dd5f5fd
Compare
| var runID int64 | ||
|
|
||
| // Keeper jobs are special in that they do not have a task graph. | ||
| if jb.Type == job.Keeper { |
There was a problem hiding this comment.
chainlink.Application#RunJobV2 is only used by the web interface to initiate one-off jobs. Seems inapplicable to Keeper jobs, especially given that there's no TaskDAG of any kind, just an insertion of a DB row. What's the thinking?
There was a problem hiding this comment.
Its actually not used by the UI at all - @DeividasK added it to help with debugging. I also just used it to be able to create local keeper runs to test. I made the debugging nature of it explicit with the CHAINLINK_DEV check
There was a problem hiding this comment.
Didn't mean the UI specifically. Was thinking-- wouldn't this be useful for the web job type? So probably good to avoid the DEV var
There was a problem hiding this comment.
Oh yeah for sure, when we add the web v2 jobs I'd imagine they could use the same logic. Maybe when that happens we'd want to break out a non-dev function/endpoint though
core/services/keeper/delegate.go
Outdated
| @@ -53,6 +57,12 @@ func (d *Delegate) ServicesForSpec(spec job.Job) (services []job.Service, err er | |||
| return nil, errors.Wrap(err, "unable to create keeper registry contract wrapper") | |||
| } | |||
|
|
|||
| // Make sure we can read the contract | |||
There was a problem hiding this comment.
Would this cause the job to fail and stop attempting to execute if the ETH node is inaccessible briefly on startup?
There was a problem hiding this comment.
Yeah it would. I was thinking you could just delete and add, but thats not great. I'll move the spec_error creation to the GetConfig call in newRegistryFromChain and make it non-blocking
| @@ -266,6 +266,28 @@ func NewConfigWithWSServer(t testing.TB, url string, wsserver *httptest.Server) | |||
| return config | |||
| } | |||
|
|
|||
| type JobPipelineV2TestHelper struct { | |||
There was a problem hiding this comment.
We should probably use this pattern more in our tests
There was a problem hiding this comment.
Agreed. I think it'll just be a slow migration
| if err != nil { | ||
| return errors.Wrap(err, "transmitter failed to get RowsAffected on eth_tx insert") | ||
| if etx.ID == 0 { | ||
| return etx, errors.New("a keeper eth_tx with insufficient eth is present, not creating a new eth_tx") |
There was a problem hiding this comment.
It's actually any tx with insufficient eth, but this message is probably ok.
We want to show runs to indicate progress on the keeper jobs as well as spec errors in the case of the contract not existing at the address specified.
Run:
Spec Error:
