Skip to content

Commit 24040ae

Browse files
acabarbayeacabarbaye
authored
✨ Improve job message fetching (#75)
<!-- Copyright (C) 2020-2022 Arm Limited or its affiliates and Contributors. All rights reserved. SPDX-License-Identifier: Proprietary --> ### Description - looks for job affordances ### Test Coverage <!-- Please put an `x` in the correct box e.g. `[x]` to indicate the testing coverage of this change. --> - [x] This change is covered by existing or additional automated tests. - [ ] Manual testing has been performed (and evidence provided) as automated testing was not feasible. - [ ] Additional tests are not required for this change (e.g. documentation update). --------- Co-authored-by: acabarbaye <[email protected]>
1 parent 141fe6e commit 24040ae

File tree

9 files changed

+146
-11
lines changed

9 files changed

+146
-11
lines changed

‎changes/20240617160218.bugfix‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
:bug: [`job`] Wait for job to start before progressing with messages

‎changes/20240618142953.feature‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
:sparkles: Improve message logging by checking job affordances

‎utils/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ module github.com/ARM-software/embedded-development-services-client-utils/utils
33
go 1.22
44

55
require (
6-
github.com/ARM-software/embedded-development-services-client/client v1.31.4
6+
github.com/ARM-software/embedded-development-services-client/client v1.32.0
77
github.com/ARM-software/golang-utils/utils v1.68.0
88
github.com/go-faker/faker/v4 v4.4.2
99
github.com/go-logr/logr v1.4.2

‎utils/go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
bitbucket.org/creachadair/stringset v0.0.9/go.mod h1:t+4WcQ4+PXTa8aQdNKe40ZP6iwesoMFWAxPGd3UGjyY=
2-
github.com/ARM-software/embedded-development-services-client/client v1.31.4 h1:sUtf53bEPaME7GBqPAFCBoVRDdAn6brZAuVnlYfsKns=
3-
github.com/ARM-software/embedded-development-services-client/client v1.31.4/go.mod h1:ZNNMwjBLtXKIUHed1p3EYy71CzFWeUv8C/HLgqlNvY0=
2+
github.com/ARM-software/embedded-development-services-client/client v1.32.0 h1:8xaBwOT/vyCsMbiOBliG1JOpSGoI6x2GcBlT23wdriU=
3+
github.com/ARM-software/embedded-development-services-client/client v1.32.0/go.mod h1:4InxEuNB10hcU2a8b8YAjCtT2jmPFqY3V/A4zuls9CU=
44
github.com/ARM-software/golang-utils/utils v1.68.0 h1:Sp92FvRhGhzfnvIKHDdLb8slrhcidBNFzB9GkpcsN3E=
55
github.com/ARM-software/golang-utils/utils v1.68.0/go.mod h1:pjIYW6jPUXH7/kxkZEPyzRwiux+NUG/BFJFn8zaJ/0A=
66
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=

‎utils/job/interfaces.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ type IAsynchronousJob interface {
3131
GetStatus() string
3232
// GetQueued returns whether the job is being queued and has not started just yet
3333
GetQueued() bool
34+
// HasMessages returns whether the job has messages available.
35+
HasMessages() bool
36+
// HasArtefacts returns whether the job has artefacts available.
37+
HasArtefacts() bool
3438
}
3539

3640
// IJobManager defines a manager of asynchronous jobs

‎utils/job/job_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Copyright (C) 2020-2024 Arm Limited or its affiliates and Contributors. All rights reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
package job
6+
7+
import (
8+
"fmt"
9+
"testing"
10+
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
14+
"github.com/ARM-software/embedded-development-services-client-utils/utils/job/jobtest"
15+
"github.com/ARM-software/embedded-development-services-client/client"
16+
)
17+
18+
func TestAsynchronousJobImplementation(t *testing.T) {
19+
mock, err := jobtest.NewMockSuccessfulAsynchronousJob()
20+
require.NoError(t, err)
21+
tests := []struct {
22+
impl any
23+
}{
24+
{
25+
impl: client.NewBuildJobItemWithDefaults(),
26+
},
27+
{
28+
impl: client.NewIntellisenseJobItemWithDefaults(),
29+
},
30+
{
31+
impl: client.NewGenericWorkJobItemWithDefaults(),
32+
},
33+
{
34+
impl: mock,
35+
},
36+
// {
37+
// impl: client.NewVhtRunJobItemWithDefaults(),
38+
// },
39+
}
40+
for i := range tests {
41+
test := tests[i]
42+
t.Run(fmt.Sprintf("%T", test.impl), func(t *testing.T) {
43+
assert.Implements(t, (*IAsynchronousJob)(nil), test.impl)
44+
})
45+
}
46+
47+
}

‎utils/job/jobtest/testing.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,11 @@ type MockAsynchronousJob struct {
1818
failure bool
1919
}
2020

21-
func (m *MockAsynchronousJob) GetQueued() bool {
22-
return !m.done
23-
}
21+
func (m *MockAsynchronousJob) HasMessages() bool { return true }
22+
23+
func (m *MockAsynchronousJob) HasArtefacts() bool { return false }
24+
25+
func (m *MockAsynchronousJob) GetQueued() bool { return !m.done }
2426

2527
func (m *MockAsynchronousJob) FetchType() string {
2628
return "Mock Asynchronous Job"

‎utils/job/manager.go‎

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func (m *Manager) FetchJobMessagesFirstPage(ctx context.Context, job IAsynchrono
5555
return
5656
}
5757

58-
func (m *Manager) waitForJobToStart(ctx context.Context, logger messages.IMessageLogger, job IAsynchronousJob) (err error) {
58+
func waitForJobState(ctx context.Context, logger messages.IMessageLogger, job IAsynchronousJob, jobState string, checkStateFunc func(context.Context, IAsynchronousJob) (bool, error)) (err error) {
5959
err = parallelisation.DetermineContextError(ctx)
6060
if err != nil {
6161
return
@@ -65,20 +65,28 @@ func (m *Manager) waitForJobToStart(ctx context.Context, logger messages.IMessag
6565
if err != nil {
6666
return
6767
}
68-
notStartedError := fmt.Errorf("%w: job [%v] has not started", commonerrors.ErrCondition, jobName)
68+
notStartedError := fmt.Errorf("%w: job [%v] has not reached the expected state [%v]", commonerrors.ErrCondition, jobName, jobState)
6969
err = retry.RetryOnError(ctx, logs.NewPlainLogrLoggerFromLoggers(logger), retry.DefaultExponentialBackoffRetryPolicyConfiguration(), func() error {
70-
started, subErr := m.HasJobStarted(ctx, job)
70+
inState, subErr := checkStateFunc(ctx, job)
7171
if subErr != nil {
7272
return subErr
7373
}
74-
if started {
74+
if inState {
7575
return nil
7676
}
7777
return notStartedError
78-
}, fmt.Sprintf("Waiting for job [%v] to start...", jobName), notStartedError)
78+
}, fmt.Sprintf("Waiting for job [%v] to %v...", jobName, jobState), notStartedError)
7979
return
8080
}
8181

82+
func (m *Manager) waitForJobToStart(ctx context.Context, logger messages.IMessageLogger, job IAsynchronousJob) error {
83+
return waitForJobState(ctx, logger, job, "start", m.HasJobStarted)
84+
}
85+
86+
func (m *Manager) waitForJobToHaveMessagesAvailable(ctx context.Context, logger messages.IMessageLogger, job IAsynchronousJob) error {
87+
return waitForJobState(ctx, logger, job, "have messages", m.areThereMessages)
88+
}
89+
8290
func (m *Manager) createMessagePaginator(ctx context.Context, job IAsynchronousJob) (paginator pagination.IStreamPaginatorAndPageFetcher, err error) {
8391
paginator, err = m.messagesPaginatorFactory.Create(ctx, func(subCtx context.Context) (pagination.IStaticPageStream, error) {
8492
return m.FetchJobMessagesFirstPage(subCtx, job)
@@ -104,6 +112,10 @@ func (m *Manager) WaitForJobCompletion(ctx context.Context, job IAsynchronousJob
104112
if err != nil {
105113
return
106114
}
115+
err = m.waitForJobToHaveMessagesAvailable(ctx, messageLogger, job)
116+
if err != nil {
117+
return
118+
}
107119
messagePaginator, err := m.createMessagePaginator(ctx, job)
108120
if err != nil {
109121
return
@@ -149,6 +161,37 @@ func (m *Manager) checkForMessageStreamExhaustion(ctx context.Context, paginator
149161
}
150162
}
151163

164+
func (m *Manager) areThereMessages(ctx context.Context, job IAsynchronousJob) (hasMessages bool, err error) {
165+
err = parallelisation.DetermineContextError(ctx)
166+
if err != nil {
167+
return
168+
}
169+
if job == nil {
170+
err = fmt.Errorf("%w: missing job", commonerrors.ErrUndefined)
171+
return
172+
}
173+
if job.HasMessages() {
174+
hasMessages = true
175+
return
176+
}
177+
178+
jobName, err := job.FetchName()
179+
if err != nil {
180+
return
181+
}
182+
jobType := job.FetchType()
183+
jobStatus, resp, apierr := m.fetchJobStatusFunc(ctx, jobName)
184+
if resp != nil {
185+
_ = resp.Body.Close()
186+
}
187+
err = api.CheckAPICallSuccess(ctx, fmt.Sprintf("could not fetch %v [%v]'s status", jobType, jobName), resp, apierr)
188+
if err != nil {
189+
return
190+
}
191+
hasMessages = jobStatus.HasMessages()
192+
return
193+
}
194+
152195
func (m *Manager) HasJobStarted(ctx context.Context, job IAsynchronousJob) (started bool, err error) {
153196
err = parallelisation.DetermineContextError(ctx)
154197
if err != nil {
@@ -158,6 +201,15 @@ func (m *Manager) HasJobStarted(ctx context.Context, job IAsynchronousJob) (star
158201
err = fmt.Errorf("%w: missing job", commonerrors.ErrUndefined)
159202
return
160203
}
204+
if job.GetDone() {
205+
started = true
206+
return
207+
}
208+
if !job.GetQueued() {
209+
started = true
210+
return
211+
}
212+
161213
jobName, err := job.FetchName()
162214
if err != nil {
163215
return

‎utils/mocks/mock_job.go‎

Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)