Skip to content

feat(scripts): Add Dockerfile linting tools#28

Merged
agreaves-ms merged 3 commits intomainfrom
feat/devcontainer-fixes
Dec 11, 2025
Merged

feat(scripts): Add Dockerfile linting tools#28
agreaves-ms merged 3 commits intomainfrom
feat/devcontainer-fixes

Conversation

@agreaves-ms
Copy link
Collaborator

Enhanced the devcontainer configuration and setup script for a more streamlined developer experience. Consolidated Python formatting to use Ruff, added NVIDIA tools (OSMO CLI, NGC CLI), and improved the setup script with better error handling and optional virtual environment support.

  • feat(devcontainer): Added Azure ML CLI extension to Azure CLI features and VS Code AI extension for machine learning workflows

  • feat(devcontainer): Consolidated Python formatting to Ruff, replacing autopep8, black-formatter, and flake8 extensions

    • Added Python editor settings for auto-fix and import organization on save
  • feat(devcontainer): Added NVIDIA tool installations in onCreateCommand

    • OSMO CLI installed from NVIDIA GitHub repository
    • NGC CLI downloaded and installed to /usr/local/bin
    • Azure ML extension added via az extension add
  • refactor(devcontainer): Removed unused extensions (Bicep, C# DevKit, pipx-package, pre-commit features)

  • feat(devcontainer): Added jq and unzip to apt packages for CLI tool support

  • feat(scripts): Rewrote setup-dev.sh with improved developer experience

    • Added --disable-venv flag for devcontainer environments
    • Added tool verification section checking for az, terraform, kubectl, helm, jq
    • Added preamble recommending devcontainer usage with setup instructions
    • Graceful fallback to system python3 when pyenv is unavailable
  • feat(scripts): Improved setup script output with colored sections using common.sh utilities

  • fix(scripts): Added quiet mode to pip installs with graceful error handling for platform-specific packages

🐳 - Generated by Copilot

…ensions and commands

- add 'ml' extension to Azure CLI
- include new Python formatting settings
- install OSMO and NGC CLI tools in onCreateCommand
- update apt installation to include jq and unzip

🔧 - Generated by Copilot
…ent setup

- recommend using devcontainer for easier setup
- add tool verification and installation steps for Azure CLI and ML extension
- improve Python environment setup with pyenv and system Python fallback
- streamline virtual environment creation and package installation
- update IsaacLab setup instructions for clarity

✅ - Generated by Copilot
…onment

- add --disable-venv option to setup-dev.sh
- modify onCreateCommand in devcontainer.json to use setup-dev script

🔧 - Generated by Copilot
@agreaves-ms agreaves-ms merged commit a930ac0 into main Dec 11, 2025
5 checks passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the development environment setup by improving the devcontainer configuration and rewriting the setup-dev.sh script for better developer experience. The changes consolidate Python tooling around Ruff, add NVIDIA-specific tools (OSMO CLI, NGC CLI), and improve the setup script with better error handling, colored output, and optional virtual environment support.

Key Changes:

  • Rewrote setup-dev.sh with improved UX: tool verification, colored output using common.sh helpers, --disable-venv flag for devcontainer environments, and graceful fallback when pyenv is unavailable
  • Consolidated Python formatting to Ruff with auto-fix on save, replacing autopep8, black-formatter, and flake8 extensions
  • Added NVIDIA tools (OSMO CLI, NGC CLI) and Azure ML extension to devcontainer onCreateCommand

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
setup-dev.sh Complete rewrite with argument parsing, tool verification section, improved error handling with quiet pip installs, colored output using common.sh utilities, and helpful preamble recommending devcontainer usage
.devcontainer/devcontainer.json Added Azure ML CLI extension, consolidated to Ruff for Python formatting with editor settings, added NVIDIA tool installations (OSMO CLI, NGC CLI), added jq/unzip packages, removed unused extensions (Bicep, C# DevKit, pipx, pre-commit, autopep8, black, flake8)

"update-bashrc": "echo 'export PATH=\"${containerWorkspaceFolder}/scripts:$PATH\"' | sudo tee -a ~/.bashrc"
"update-bashrc": "echo 'export PATH=\"${containerWorkspaceFolder}/scripts:$PATH\"' | sudo tee -a ~/.bashrc",
"osmo-cli": "curl -fsSL https://raw.githubusercontent.com/NVIDIA/OSMO/refs/heads/main/install.sh | bash || true",
"ngc-cli": "wget -qO /tmp/ngccli.zip https://ngc.nvidia.com/downloads/ngccli_linux.zip && sudo unzip -o /tmp/ngccli.zip -d /usr/local/bin && rm /tmp/ngccli.zip && sudo chmod u+x /usr/local/bin/ngc || true",
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The NGC CLI installation uses sudo chmod u+x which only sets execute permission for the owner. Since the file is installed to /usr/local/bin (a system directory), it should use sudo chmod +x to ensure the executable is runnable by all users. The current approach may prevent other users from executing the ngc command.

Suggested change
"ngc-cli": "wget -qO /tmp/ngccli.zip https://ngc.nvidia.com/downloads/ngccli_linux.zip && sudo unzip -o /tmp/ngccli.zip -d /usr/local/bin && rm /tmp/ngccli.zip && sudo chmod u+x /usr/local/bin/ngc || true",
"ngc-cli": "wget -qO /tmp/ngccli.zip https://ngc.nvidia.com/downloads/ngccli_linux.zip && sudo unzip -o /tmp/ngccli.zip -d /usr/local/bin && rm /tmp/ngccli.zip && sudo chmod +x /usr/local/bin/ngc || true",

Copilot uses AI. Check for mistakes.
Comment on lines +80 to +82
if ! pip install -r "${SCRIPT_DIR}/requirements.txt" --quiet 2>/dev/null; then
warn "Some packages failed to install (expected on macOS for Linux-only packages)"
fi
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The pip install commands redirect stderr to /dev/null (2>/dev/null), which completely suppresses error messages. This makes debugging difficult when installations fail. Consider redirecting stderr to a log file or at least letting critical errors through while only suppressing warnings. This would help users understand what went wrong during setup.

Copilot uses AI. Check for mistakes.
Comment on lines +85 to +87
if ! pip install -r "${SCRIPT_DIR}/src/training/requirements.txt" --quiet 2>/dev/null; then
warn "Some training packages failed to install"
fi
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

Similar to the root dependencies installation, stderr is completely suppressed with 2>/dev/null, hiding potentially useful error information. Consider redirecting to a log file or showing critical errors to help with debugging installation issues.

Copilot uses AI. Check for mistakes.
echo "✅ Development environment setup complete!"
echo
if [[ "${DISABLE_VENV}" == "false" ]]; then
warn "Run this command to activate the virtual environment:"
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The warning message says "Run this command to activate the virtual environment" but uses the warn function which outputs to stderr with a yellow [WARN] prefix. This is more of an informational message than a warning. Consider using info instead, or use plain echo for this instructional message.

Suggested change
warn "Run this command to activate the virtual environment:"
info "Run this command to activate the virtual environment:"

Copilot uses AI. Check for mistakes.
echo "Virtual environment already exists at ${VENV_DIR}"
if [[ ! -d "${VENV_DIR}" ]]; then
info "Creating virtual environment at ${VENV_DIR}..."
$PYTHON_CMD -m venv "${VENV_DIR}"
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The variable $PYTHON_CMD should be quoted to prevent word splitting in case the path contains spaces. While unlikely for Python executables, it's a best practice for shell scripts to quote variable expansions. Change to "$PYTHON_CMD".

Suggested change
$PYTHON_CMD -m venv "${VENV_DIR}"
"$PYTHON_CMD" -m venv "${VENV_DIR}"

Copilot uses AI. Check for mistakes.
@WilliamBerryiii WilliamBerryiii changed the title feat(devcontainer): enhance development environment setup feat(scripts): Add Dockerfile linting tools Feb 5, 2026
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