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

feat(docker): no longer download artifacts on devel image #5023

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

youtalk
Copy link
Member

@youtalk youtalk commented Jul 23, 2024

Description

https://github.com/orgs/autowarefoundation/discussions/5007#discussioncomment-10086717

Considering these, I would propose to have an image without artifacts for development and CI purposes.
Version with artifacts can be something that derives from this light image.

Based on this opinion, this PR excludes the artifacts from the devel image. The artifacts still continues to be included in the runtime image.

Tests performed

https://github.com/autowarefoundation/autoware/actions/runs/10052443863

Effects on system behavior

Not applicable.

Interface changes

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

@youtalk youtalk self-assigned this Jul 23, 2024
@youtalk youtalk added type:github-actions Github actions related. (auto-assigned) type:containers Docker containers, containerization of components, or container orchestration. tag:run-health-check Run health-check labels Jul 23, 2024
@youtalk youtalk marked this pull request as ready for review July 23, 2024 03:58
@youtalk youtalk requested a review from oguzkaganozt as a code owner July 23, 2024 03:58
@youtalk youtalk requested review from xmfcx and mitsudome-r July 23, 2024 03:59
@mitsudome-r
Copy link
Member

I'm okay with removing the artifact from the image, but do you have any idea about how the users run ML based nodes with docker?

  1. Users download the artifact every time they start up a new container
  2. Users download the artifact on host and mount when they run the container

I prefer option 2, but in that case, I think we should be updating the instructions in Autoware Documentation as well to tell the user to download the artifacts and mount them with run.sh script if they wish to use perception nodes. https://autowarefoundation.github.io/autoware-documentation/main/installation/autoware/docker-installation/#runtime-setup

@youtalk
Copy link
Member Author

youtalk commented Jul 23, 2024

@mitsudome-r These reverted PRs need to revert again for the developers.
#4864
autowarefoundation/autoware-documentation#571

Note that the runtime image still includes the artifacts.

@youtalk youtalk merged commit 8979744 into main Jul 23, 2024
38 checks passed
@youtalk youtalk deleted the not-download-artifacts-on-devel branch July 23, 2024 13:13
@xmfcx
Copy link
Contributor

xmfcx commented Jul 23, 2024

This PR won't help CI because we are using autoware-universe image in CI, not devel.
And autoware-universe images didn't have the artifacts in the first place.

Also posted here:

@youtalk
Copy link
Member Author

youtalk commented Jul 23, 2024

Hmm, you mean we don't need this PR?

@xmfcx
Copy link
Contributor

xmfcx commented Jul 23, 2024

I think it's ok to keep this PR because developers will probably prefer keeping the artifacts outside the image.

But it won't help CI since we use another image for it.

youtalk added a commit to youtalk/autoware that referenced this pull request Jul 24, 2024
* feat(ci): disable `cache-to` option to run `health-check` from forked branch (autowarefoundation#5021)

disable cache-to

Signed-off-by: Yutaka Kondo <[email protected]>

* feat: add single lidar sensor kit (autowarefoundation#4988)

feat(autoware.repos): add single lidar sensor kit

Signed-off-by: beginningfan <[email protected]>
Co-authored-by: Yutaka Kondo <[email protected]>

* fix(docker-build): fix ccache typo (autowarefoundation#5024)

Signed-off-by: mitsudome-r <[email protected]>

* feat(docker): no longer download artifacts on `devel` image (autowarefoundation#5023)

not download artifacts on devel

Signed-off-by: Yutaka Kondo <[email protected]>

---------

Signed-off-by: Yutaka Kondo <[email protected]>
Signed-off-by: beginningfan <[email protected]>
Signed-off-by: mitsudome-r <[email protected]>
Co-authored-by: beginningfan <[email protected]>
Co-authored-by: Ryohsuke Mitsudome <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag:run-health-check Run health-check type:containers Docker containers, containerization of components, or container orchestration. type:github-actions Github actions related. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants