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.

Sign up for GitHub

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

Jump to bottom

feat(docker,ansible): revert "feat(docker,ansible): download artifacts at runtime (#4771)" #4864

Merged
merged 1 commit into from
Jun 13, 2024
Merged

feat(docker,ansible): revert "feat(docker,ansible): download artifacts at runtime (#4771)" #4864

merged 1 commit into from
Jun 13, 2024

Conversation

youtalk
Copy link
Member

@youtalk youtalk commented Jun 13, 2024

This reverts #4771 according to #4849 (comment).

It's even better than before! We can review it easier and faster.
Do you have an actual reason to re-add --download-artifacts at build time? ref. #4771

I think we should keep our images as complete as possible. Since we don't have any disk usage problem anymore, we can re-add those artifacts to simplify container usage. Because the original intention was to save space.

And also think about a scenario in which user just downloads the image and go on a field test where there is no sufficient internet connection to download all artifacts. IMHO, devel and runtime images should be self-sufficient. Moreover these artifacts are mandatory to run essential parts of the Autoware(perception, localization) that means user will always need to download them so why not just keep them inside the image ?

@youtalk
Revert "feat(docker,ansible): download artifacts at runtime (#4771)"
52bbe2a 
This reverts commit e8a258f.

Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
@youtalk youtalk mentioned this pull request Jun 13, 2024
Merged
4 tasks
@youtalk youtalk enabled auto-merge (squash) June 13, 2024 02:45
oguzkaganozt
oguzkaganozt approved these changes Jun 13, 2024
View reviewed changes
Copy link
Contributor

@oguzkaganozt oguzkaganozt left a comment

Choose a reason for hiding this comment

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

LGTM

@youtalk youtalk merged commit bd66903 into main Jun 13, 2024
12 of 13 checks passed
@youtalk youtalk deleted the revert-disable-download-artifacts branch June 13, 2024 11:04
@tier4-autoware-public-bot tier4-autoware-public-bot bot mentioned this pull request Jun 14, 2024
Open
@youtalk youtalk mentioned this pull request Jun 24, 2024
Merged
pravinkmr26 pushed a commit to pravinkmr26/autoware that referenced this pull request Jul 15, 2024
@youtalk @pravinkmr26
feat(docker,ansible): revert "feat(docker,ansible): download artifact…
3fb1ff0 
…s at runtime (autowarefoundation#4771)" (autowarefoundation#4864)

Revert "feat(docker,ansible): download artifacts at runtime (autowarefoundation#4771)"

This reverts commit e8a258f.

Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
@youtalk youtalk mentioned this pull request Jul 23, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oguzkaganozt oguzkaganozt oguzkaganozt approved these changes

@mitsudome-r mitsudome-r Awaiting requested review from mitsudome-r mitsudome-r is a code owner

@xmfcx xmfcx Awaiting requested review from xmfcx xmfcx is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@youtalk @oguzkaganozt