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

[OHI-1547] fix(docker): run compression regardless of APP_CONFIG being present ( in cases such as volume mount) #4673

Merged
merged 2 commits into from
Jan 10, 2025

Conversation

IbrahimCSAE
Copy link
Collaborator

@IbrahimCSAE IbrahimCSAE commented Jan 10, 2025

Context

Docker compression step was not being triggered if APP_CONFIG is not present, but it should still do if a user mounts a file using volumes.

Fixes OHIF-1547
Closes #4669

Copy link

netlify bot commented Jan 10, 2025

Deploy Preview for ohif-platform-docs canceled.

Name Link
🔨 Latest commit ba3c79b
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-docs/deploys/67814f12679dd90008212702

Copy link

netlify bot commented Jan 10, 2025

Deploy Preview for ohif-dev canceled.

Name Link
🔨 Latest commit ba3c79b
🔍 Latest deploy log https://app.netlify.com/sites/ohif-dev/deploys/67814f128d1f800008fabad1

Copy link

cypress bot commented Jan 10, 2025

Viewers    Run #4669

Run Properties:  status check passed Passed #4669  •  git commit ba3c79ba12: updates
Project Viewers
Branch Review fix/docker
Run status status check passed Passed #4669
Run duration 02m 08s
Commit git commit ba3c79ba12: updates
Committer Ibrahim
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 44
View all changes introduced in this branch ↗︎

echo "Not using custom app config"

if [ -f /usr/share/nginx/html${PUBLIC_URL}app-config.js ]; then
if [ -s /usr/share/nginx/html${PUBLIC_URL}app-config.js ]; then
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

check if empty

then
echo "Not using custom app config"

if [ -f /usr/share/nginx/html${PUBLIC_URL}app-config.js ]; then
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

check if exists

@IbrahimCSAE IbrahimCSAE changed the title fix(docker): run compression regardless of APP_CONFIG being present ( in cases such as volume mount) [OHI-1547] fix(docker): run compression regardless of APP_CONFIG being present ( in cases such as volume mount) Jan 10, 2025
@wayfarer3130
Copy link
Contributor

Thank you for fixing that.
Could you update the documentation for the PUBLIC_URL to indicate that it is no longer necessary or desirable to update the routerBasename variable? That was also changed as part of the base issue here, and would be good to address.

@IbrahimCSAE
Copy link
Collaborator Author

IbrahimCSAE commented Jan 10, 2025

Thank you for fixing that. Could you update the documentation for the PUBLIC_URL to indicate that it is no longer necessary or desirable to update the routerBasename variable? That was also changed as part of the base issue here, and would be good to address.

Yes, let me do that, thanks Bill

Copy link
Member

@sedghi sedghi left a comment

Choose a reason for hiding this comment

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

Thanks

@sedghi sedghi merged commit a2d34c9 into master Jan 10, 2025
20 checks passed
@sedghi
Copy link
Member

sedghi commented Jan 10, 2025

I merged too quick sorry

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.

[Bug] Unable to load app-config.js via docker volume mount since PR#4559 / v3.10.0-beta.34 - demo data showing
3 participants