Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: Remove duplicate docker-build job #5327

Merged
merged 3 commits into from
May 6, 2024
Merged

ci: Remove duplicate docker-build job #5327

merged 3 commits into from
May 6, 2024

Conversation

pingsutw
Copy link
Member

@pingsutw pingsutw commented May 6, 2024

Tracking issue

NA

Why are the changes needed?

We built the same docker image two times in the CI.

What changes were proposed in this pull request?

Remove duplicate docker-build job

How was this patch tested?

CI

Setup process

Screenshots

https://github.com/flyteorg/flyte/actions/runs/8965665995/job/24619590033

Screenshot 2024-05-06 at 4 00 26 PM

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

NA

Docs link

NA

Copy link

codecov bot commented May 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.08%. Comparing base (0a1c82c) to head (a67a776).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5327      +/-   ##
==========================================
+ Coverage   61.07%   61.08%   +0.01%     
==========================================
  Files         794      794              
  Lines       51203    51203              
==========================================
+ Hits        31271    31277       +6     
+ Misses      17049    17043       -6     
  Partials     2883     2883              
Flag Coverage Δ
unittests-datacatalog 69.31% <ø> (ø)
unittests-flyteadmin 58.82% <ø> (+0.04%) ⬆️
unittests-flytecopilot 17.79% <ø> (ø)
unittests-flytectl 68.30% <ø> (ø)
unittests-flyteidl 79.30% <ø> (ø)
unittests-flyteplugins 61.94% <ø> (ø)
unittests-flytepropeller 57.32% <ø> (ø)
unittests-flytestdlib 65.75% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pingsutw added 2 commits May 6, 2024 17:24
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Copy link
Contributor

@eapolinario eapolinario left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

@eapolinario eapolinario merged commit dd022b5 into master May 6, 2024
46 checks passed
@eapolinario eapolinario deleted the duplicate-build branch May 6, 2024 17:09
@eapolinario
Copy link
Contributor

This was added there to exercise the codepath used during release (notice how we only push to ghcr in a release).

If you read the gh job integration, build-docker is a pre-req, but only for the flyteadmin integration tests here. What do you think of the idea of (1) restricting build-docker to only build the flyte-admin image and (2) create a separate gh issue to run the flyte-admin integration tests using single-binary in dev mode? (1) is expressed in #5331.

austin362667 pushed a commit to austin362667/flyte that referenced this pull request May 7, 2024
* ci: Remove duplicate docker-build job

Signed-off-by: Kevin Su <[email protected]>

* fix-test

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

---------

Signed-off-by: Kevin Su <[email protected]>
robert-ulbrich-mercedes-benz pushed a commit to robert-ulbrich-mercedes-benz/flyte that referenced this pull request Jul 2, 2024
* ci: Remove duplicate docker-build job

Signed-off-by: Kevin Su <[email protected]>

* fix-test

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

---------

Signed-off-by: Kevin Su <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants