Skip to content

Conversation

@SteveDala
Copy link

@SteveDala SteveDala commented Jan 25, 2026

Description

Fixes an issue where the image build would sometimes fail non-interactively due to a missing -y tag in the Dockerfile. Also improves the container so that Thoth Tech and Doubtfire Astro containers can be run at the same time by changing conflicting ports and Docker volumes.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Documentation (update or new)

How Has This Been Tested?

  1. Open the repository folder in VSCode. VSCode should have the "Remote Development Extension Pack" installed.
  2. Using the command palette (Ctrl + Shift + P on Windows), use the command "Reopen in Container".
  3. It may take some time to build the container, but before long the Astro dev server should start within the container and become available at http://localhost:4322 in any browser window.
  4. When saving edits to any documentation, Vite should detect the change and the page should update.

Testing Checklist

  • Tested in latest Docker

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have requested a review from ... on the Pull Request

@netlify
Copy link

netlify bot commented Jan 25, 2026

Deploy Preview for ontrackdocumentation ready!

Name Link
🔨 Latest commit 60ceea8
🔍 Latest deploy log https://app.netlify.com/projects/ontrackdocumentation/deploys/6976bd1357f407000828d5ed
😎 Deploy Preview https://deploy-preview-68--ontrackdocumentation.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@SteveDala SteveDala marked this pull request as ready for review January 25, 2026 07:36
Copy link

@ishika021 ishika021 left a comment

Choose a reason for hiding this comment

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

I reviewed the improvements made to the devcontainer and spent some time going through the normal flows related to the new devcontainer service and ports that have been changed. Overall, I think the direction taken is quite encouraging (i.e., having a dedicated df-astro-dev devcontainer service, as well as a devcontainer-friendly container lifecycle) and I also find the change to the Dockerfile, designed to make upgrading non-interactive, is beneficial.

Points of interest:

  • Devcontainer is now targeted toward df-astro-dev, where 4322 is forwarded.

  • The devcontainer has a typical compose setup pattern of bind mounting a directory with a sleep infinity process.

  • The Dockerfile uses apt-get upgrade -y to create non-interactive builds.

Points to address and/or verify prior to the merge:

  • There appears to be a blank service definition in the docker-compose.yml for an astro-dev: service key. Is this intentional or a holdover?

  • The /site/node_modules directory is mounted with two named volumes. As this could cause inconsistencies in dependency behavior, I recommend only having one node_modules volume mapping.

  • The devcontainer service uses both 4321 and 4322 as its port, dockerfile uses 4322, and the compose file uses 4321. I recommend standardizing all three to utilize one port (probably 4322) and removing all references to other ports in the environment as well as unset the mappings/env/expose entries for 4321.

Overall, everything looks to be going in the right direction.

@SteveDala
Copy link
Author

Hi @ishika021,

While I appreciate your comment, I don't appreciate it entirely being generated by an LLM.

  • There appears to be a blank service definition in the docker-compose.yml for an astro-dev: service key. Is this intentional or a holdover?

Please take another look at the diff. astro-dev is the old service. It is being removed in favour of df-astro-dev. The only way this mistake could be made is if the diff sheet was placed into an LLM and the output uncritically posted.

  • The /site/node_modules directory is mounted with two named volumes. As this could cause inconsistencies in dependency behavior, I recommend only having one node_modules volume mapping.

Again, no. The only way this could be mistaken is if the diff was placed into an LLM. The old mapping is astro-node_modules:/site/node_modules and the new mapping is df-astro-node_modules:/site/node_modules.

  • The devcontainer service uses both 4321 and 4322 as its port, dockerfile uses 4322, and the compose file uses 4321. I recommend standardizing all three to utilize one port (probably 4322) and removing all references to other ports in the environment as well as unset the mappings/env/expose entries for 4321.

Again, no - the diff shows that 4322 is the new port, while 4321 is the old port.

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