Reference task to fetch latest version by default #5895
Conversation
|
Thank you for opening this pull request! 🙌 These tips will help get your PR across the finish line:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5895 +/- ##
=======================================
Coverage 36.85% 36.86%
=======================================
Files 1310 1310
Lines 131246 131270 +24
=======================================
+ Hits 48377 48395 +18
- Misses 78670 78676 +6
Partials 4199 4199
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
|
featherchen
force-pushed
the
issue-5825
branch
from
a06323d to
044dd03
Compare
|
@eapolinario TestGetTask should work now, thanks @troychiu for the fix. |
Signed-off-by: featherchen <[email protected]>
Signed-off-by: featherchen <[email protected]>
Signed-off-by: featherchen <[email protected]>
Signed-off-by: featherchen <[email protected]>
featherchen
force-pushed
the
issue-5825
branch
6 times, most recently
from
35f91cc to
7ff7fdc
Compare
featherchen
changed the title
Signed-off-by: featherchen <[email protected]>
featherchen
force-pushed
the
issue-5825
branch
from
7ff7fdc to
fb6f97d
Compare
|
@troychiu Can you help me review this PR? Thanks. |
| }).Take(&task) | ||
| var tx *gorm.DB | ||
| if input.Version == "" { | ||
| tx = r.db.WithContext(ctx).Where(`"tasks"."project" = ? AND "tasks"."domain" = ? AND "tasks"."name" = ?`, input.Project, input.Domain, input.Name).Limit(1) |
There was a problem hiding this comment.
Just curious, are we able to use TaskKey here?
models.TaskKey{
Project: input.Project,
Domain: input.Domain,
Name: input.Name,
}
| var tx *gorm.DB | ||
| if input.Version == "" { | ||
| tx = r.db.WithContext(ctx).Where(`"tasks"."project" = ? AND "tasks"."domain" = ? AND "tasks"."name" = ?`, input.Project, input.Domain, input.Name).Limit(1) | ||
| tx = tx.Order(`"tasks"."version" DESC`) |
There was a problem hiding this comment.
Why do we order by version instead of time information such as created_at?
| @@ -45,6 +45,37 @@ var taskExecutionIdentifier = &core.TaskExecutionIdentifier{ | |||
| RetryAttempt: 1, | |||
| } | |||
|
|
|||
| func TestGetTaskExecutions(t *testing.T) { | |||
There was a problem hiding this comment.
Can we register more than one tasks and see if we get the latest one?
|
cc @featherchen any updates? |
Tracking issue
Related to #5825
Why are the changes needed?
When task version is not specified (i.e. ""), we should return the latest one when
getTaskis calledWhat changes were proposed in this pull request?
When the version is "", we fetch DB directly to get the latest task.
Also remove the UT that check whether version is empty, since this situation is allowed now.
How was this patch tested?
By unit test (testGetTask)
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link