Ensure that PythonAutoContainerTask has a container_image attribute #2527
Conversation
pingsutw
requested review from
wild-endeavor,
kumare3,
eapolinario,
cosmicBboy and
samhita-alla
as code owners
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
pingsutw
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2527 +/- ##
===========================================
+ Coverage 52.89% 78.57% +25.68%
===========================================
Files 297 182 -115
Lines 23957 18567 -5390
Branches 3655 3655
===========================================
+ Hits 12671 14589 +1918
+ Misses 11178 3319 -7859
- Partials 108 659 +551 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
There was a problem hiding this comment.
I agree in principle with this PR, but I'd like to understand a bit more how this started failing all of a sudden (esp. after #2485).
| @@ -1,7 +1,7 @@ | |||
| name: Monodocs Build | |||
|
|
|||
| concurrency: | |||
| group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} | |||
| group: ${{ github.workflow }}-${{ github.event.pull_request.number }} | |||
There was a problem hiding this comment.
why are we removing this?
There was a problem hiding this comment.
It allows the GA workflow to run against all pushed commits without being canceled when a new PR is merged
There was a problem hiding this comment.
Makes sense. We use this pattern in a bunch of our repos, can you also make sure these are changed?
…lyteorg#2527) Signed-off-by: Kevin Su <pingsutw@apache.org> Signed-off-by: bugra.gedik <bugra.gedik@predera.ai>
…2527) Signed-off-by: Kevin Su <pingsutw@apache.org> Signed-off-by: Jan Fiedler <jan@union.ai>
…lyteorg#2527) Signed-off-by: Kevin Su <pingsutw@apache.org> Signed-off-by: mao3267 <chenvincent610@gmail.com>
Tracking issue
Why are the changes needed?
Fix unit test. https://github.com/flyteorg/flytekit/actions/runs/9620004661/job/26621714719
What changes were proposed in this pull request?
Call
super().__init__after setting container_image because PythonAutoContainerTaskis added to the FlyteEntities in
super().__init__. The translator will iterate overFlyteEntities and call container_image. Therefore, we should ensure the container_image attribute is set
before appending the task to FlyteEntities.
flytekit/flytekit/tools/translator.py
Line 181 in 876877a
How was this patch tested?
unit test
Setup process
Screenshots
Check all the applicable boxes
Related PRs
NA
Docs link
NA