Skip to content

refactor(dev-env): use isContainerRunning for import and sync commands#2489

Merged
sjinks merged 1 commit intotrunkfrom
fix/is-up
Aug 6, 2025
Merged

refactor(dev-env): use isContainerRunning for import and sync commands#2489
sjinks merged 1 commit intotrunkfrom
fix/is-up

Conversation

@sjinks
Copy link
Copy Markdown
Member

@sjinks sjinks commented Aug 6, 2025

Description

This pull request refactors the code to check if the development environment is running before importing SQL data. Instead of checking if the whole environment is up, it now specifically checks if both the php and database containers are running. Additionally, it introduces a new utility function to check the running status of individual containers and updates imports accordingly.

Improvements to environment status checks:

  • Replaced the broad isEnvUp check with explicit checks for the php and database containers using the new isContainerRunning function in DevEnvImportSQLCommand. This ensures a more accurate validation of required services before proceeding.

New utility function:

  • Added isContainerRunning to dev-environment-lando.ts, which checks if a specific container for a given environment is running by querying Docker directly.

Code cleanup and import updates:

  • Updated imports in dev-env-import-sql.ts and dev-environment-lando.ts to remove unused functions and add the new utility where needed. [1] [2]

Changelog Description

Changed

  • The environment status checks for vip dev-env import sql and vip dev-env sync sql commands now verify that the PHP and Database containers are running, rather than checking the status of the entire environment.

Pull request checklist

New release checklist

Steps to Test

vip dev-env create --slug test-is-up < /dev/null
vip dev-env start --slug test-is-up
docker stop testisup-nginx-1 # or whatever the container is called
vip @app.env dev-env sync sql

Without this patch, synchronization will fail with the "Environment needs to be started first" message. With this patch, it will succeed.

Replace the general `isEnvUp()` function with service-specific container status checks using Docker API. This provides more precise control over environment validation by checking the `php` and `database` containers individually.
@sjinks sjinks requested a review from Copilot August 6, 2025 20:30
@sjinks sjinks self-assigned this Aug 6, 2025
@sjinks sjinks added the [Type] Enhancement New feature or request label Aug 6, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 6, 2025

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Aug 6, 2025

Copy link
Copy Markdown
Contributor

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 pull request refactors the development environment status checks for SQL import and sync commands to be more specific and accurate. Instead of checking if the entire environment is up, it now verifies that only the required containers (PHP and database) are running before proceeding with operations.

  • Replaces broad isEnvUp checks with specific container status validation
  • Introduces a new isContainerRunning utility function for granular container status checking
  • Updates import statements to remove unused functions and add new utilities

Reviewed Changes

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

File Description
src/lib/dev-environment/dev-environment-lando.ts Adds new isContainerRunning function to check individual container status via Docker API
src/commands/dev-env-import-sql.ts Replaces isEnvUp with specific PHP and database container checks using the new utility function

@sjinks sjinks merged commit 838846f into trunk Aug 6, 2025
19 checks passed
@sjinks sjinks deleted the fix/is-up branch August 6, 2025 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants