Skip to content

WIP: Add docker-compose.yml and fix nginx paths for resources (Issue #211) - #212

Open
ajlennon wants to merge 2 commits into
Hubs-Foundation:masterfrom
DynamicDevices:master
Open

WIP: Add docker-compose.yml and fix nginx paths for resources (Issue #211)#212
ajlennon wants to merge 2 commits into
Hubs-Foundation:masterfrom
DynamicDevices:master

Conversation

@ajlennon

Copy link
Copy Markdown

What?

Allows a user to locally build and run a docker container hosting the documentation webserver

Why?

Makes it easy to build and run the documentation server locally without making host OS changes

Examples

How to test

docker compose build
docker compose up

Then browse to localhost:8080/docs

NOTE I don't know if these changes break the CI deployment to the official server

Documentation of functionality

N/A

Limitations

Might impact CI deployment

Alternatives considered

Open questions

Does this impact CI deployment

Additional details or related context

@ajlennon ajlennon changed the title WIP: Add docker-compose.yml and fix nginx paths for resources WIP: Add docker-compose.yml and fix nginx paths for resources (Issue #211) Apr 21, 2025
@DougReeder

Copy link
Copy Markdown
Member

An unfortunate complication is that Hubs in some places refers to /docs/ URLs. I think the current image would be great running on a Hubs instance, and we want to keep that. However, like you, we'd like it to be able to run independently as well.

@Exairnous Exairnous left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe the issue with building the Docker image is because of us moving away from using /docs/. This was introduced with #203 but no one thought about updating the Docker image or its nginx configuration. So I think that rather than adding more redirects for /docs/ we should be removing them. Actually, it looks like modifying the initial redirect to the following should make everything work again (and no other redirects should be needed):

location / {
        root /var/www/docs;
        autoindex off;
        add_header 'Access-Control-Allow-Origin' '*';
    }

@DougReeder Does the above seem plausible to you and in line with your changes?

Comment thread docker-compose.yml
hubs-docs:
build: .
ports:
- "8080:8080"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider whethere exposing the server on port 80 would make sense.

@DougReeder

Copy link
Copy Markdown
Member
location / {
        root /var/www/docs;
        autoindex off;
        add_header 'Access-Control-Allow-Origin' '*';
    }

That's cleaner.

I think it should be root /var/www;, please test that.

@Exairnous

Copy link
Copy Markdown
Member

Using /var/www returns a 403 forbidden error if you go to localhost:8080 and if you go to localhost:8080/docs the welcome page will display, but it will fail to find the css, js, ico files, etc.. If you exec into the container, you'll see that everything is housed within /var/www/docs. So more changes would be required to shift everything to the www folder, if we wanted to do that, but I don't think grouping everything within a docs subfolder in the server is a bad thing. I think we just want to avoid the docs subdirectory in the URL?

Exairnous added a commit to Exairnous/hubs-docs that referenced this pull request Jan 1, 2026
What:
Added the docker-compose.yml from PR 212, but instead of adding further redirects for `/docs/` to the nginx.config the root location is set to go to the internal /var/www/docs.

Why:
This keeps it in line with the changes introduced in PR Hubs-Foundation#203 and is a simpler solution.

Note:
This has already been communicated to PR Hubs-Foundation#212, so this commit is only for archival purposes.
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.

3 participants