Skip to content

Fix unexpected /app shadowing in compose.yaml (#7433) #7434

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ongrid
Copy link

@ongrid ongrid commented May 5, 2025

Paths within the container got overwritten by volume mount, which caused inconsistencies with the expected contents of directories populated on buid stage.

For example, CSS and graphics may not load — see issue #7433.

What type of PR is this?

  • Refactor
  • Feature
  • Bug Fix
  • New Query Runner (Data Source)
  • New Alert Destination
  • Other

Description

How is this tested?

  • Unit tests (pytest, jest)
  • E2E Tests (Cypress)
  • Manually
  • N/A
make compose_build
make create_database
make up

Related Tickets & Documents

#7433

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Paths within the container got overwritten by volume mount, which caused inconsistencies
with the expected contents of directories populated on buid stage.

For example, CSS and graphics may not load — see issue getredash#7433.
@@ -5,8 +5,6 @@ x-redash-service: &redash-service
context: .
args:
skip_frontend_build: "true" # set to empty string to build
volumes:
- .:/app
Copy link
Collaborator

Choose a reason for hiding this comment

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

This bindmount is not a mistake, it is required for live-reload in a dev environment

Copy link
Author

Choose a reason for hiding this comment

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

@eradman what about using separate path for frontend artifacts like /www/html here and here to avoid shadowing by mounted volume? Let me know if you accept this approach

Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to understand #7433 better. What is the reason for setting skip_frontend_build: ""?

Copy link
Author

Choose a reason for hiding this comment

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

To avoid skipping of frontend build = force frontend building. Since there is no frontend after cloning the repo, the user needs to build it first.

This is suggested in compose.yml comments

    args:
      skip_frontend_build: "true"  # set to empty string to build

BTW finding this trick was another troubleshooting journey on the way to make it run )

@ongrid ongrid marked this pull request as draft May 7, 2025 11:44
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