Skip to content

Commit

Permalink
fix: ascii encoding for subprocess output (closes #81)
Browse files Browse the repository at this point in the history
  • Loading branch information
Justintime50 committed Oct 9, 2023
1 parent 355d02c commit 95ca2a4
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 34 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# CHANGELOG

## Next Release

- Fixes a bug that couldn't deserialize subprocess output correctly due to ascii encoding (closes #81)

## v1.0.1 (2023-09-05)

- Fixes a bug where `page_size` for the `/projects` endpoint wasn't respected
Expand Down
13 changes: 5 additions & 8 deletions harvey/app.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import os
import subprocess # nosec
import threading
from typing import (
Any,
Expand Down Expand Up @@ -34,7 +33,10 @@
from harvey.repos.locks import retrieve_locks
from harvey.repos.projects import retrieve_projects
from harvey.repos.webhooks import retrieve_webhook
from harvey.utils.utils import setup_logger
from harvey.utils.utils import (
run_subprocess_command,
setup_logger,
)


# Must remain at the top of this file to take effect, we load both from the path and locally
Expand All @@ -60,12 +62,7 @@ def bootstrap() -> bool:
debug = _get_debug_bool(Config.log_level)

# Ensure the correct Docker Compose version is available
docker_compose_version = subprocess.check_output( # nosec
['docker', 'compose', 'version'],
stderr=subprocess.STDOUT,
text=True,
timeout=3,
)
docker_compose_version = run_subprocess_command(['docker', 'compose', 'version'])
if REQUIRED_DOCKER_COMPOSE_VERSION not in docker_compose_version:
raise HarveyError(f'Harvey requires Docker Compose {REQUIRED_DOCKER_COMPOSE_VERSION}.')

Expand Down
12 changes: 5 additions & 7 deletions harvey/deployments.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@
kill_deployment,
succeed_deployment,
)
from harvey.utils.utils import get_utc_timestamp
from harvey.utils.utils import (
get_utc_timestamp,
run_subprocess_command,
)
from harvey.webhooks import Webhook


Expand Down Expand Up @@ -264,12 +267,7 @@ def deploy(config: Dict[str, Any], webhook: Dict[str, Any], output: str) -> str:
# fmt: on

try:
compose_output = subprocess.check_output( # nosec
compose_command,
stderr=subprocess.STDOUT,
text=True,
timeout=Config.operation_timeout,
)
compose_output = run_subprocess_command(compose_command)
execution_time = f'Deploy stage execution time: {get_utc_timestamp() - start_time}'
final_output = f'{compose_output}\n{execution_time}'
logger.info(final_output)
Expand Down
25 changes: 6 additions & 19 deletions harvey/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
from typing import (
Any,
Dict,
List,
)

import woodchips

from harvey.config import Config
from harvey.utils.deployments import kill_deployment
from harvey.utils.utils import run_subprocess_command
from harvey.webhooks import Webhook


Expand All @@ -33,8 +33,9 @@ def clone_repo(project_path: str, webhook: Dict[str, Any]) -> str:
command_output = ''

try:
command = ['git', 'clone', '--depth=1', Webhook.repo_url(webhook), project_path]
command_output = Git._git_subprocess(command)
command_output = run_subprocess_command(
['git', 'clone', '--depth=1', Webhook.repo_url(webhook), project_path]
)
logger.debug(command_output)
except subprocess.TimeoutExpired:
kill_deployment(
Expand All @@ -57,8 +58,7 @@ def pull_repo(project_path: str, webhook: Dict[str, Any], pull_attempt: int = 1)
command_output = ''

try:
command = ['git', '-C', project_path, 'pull', '--rebase']
command_output = Git._git_subprocess(command)
command_output = run_subprocess_command(['git', '-C', project_path, 'pull', '--rebase'])
logger.debug(f'{command_output}')
except subprocess.TimeoutExpired:
kill_deployment(
Expand All @@ -72,8 +72,7 @@ def pull_repo(project_path: str, webhook: Dict[str, Any], pull_attempt: int = 1)
logger.info(f'Attempting to stash {Webhook.repo_full_name(webhook)} before pulling again...')

try:
command = ['git', '-C', project_path, 'stash']
command_output = Git._git_subprocess(command)
command_output = run_subprocess_command(['git', '-C', project_path, 'stash'])
logger.debug(f'{command_output}')
except (subprocess.CalledProcessError, subprocess.TimeoutExpired):
kill_deployment(
Expand All @@ -88,15 +87,3 @@ def pull_repo(project_path: str, webhook: Dict[str, Any], pull_attempt: int = 1)
command_output = Git.pull_repo(project_path, webhook, pull_attempt)

return command_output

@staticmethod
def _git_subprocess(command: List[str]) -> str:
"""Runs a git command via subprocess."""
command_output = subprocess.check_output( # nosec
command,
stderr=subprocess.STDOUT,
text=True,
timeout=Config.operation_timeout,
)

return command_output
15 changes: 15 additions & 0 deletions harvey/utils/utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import datetime
import subprocess # nosec
from typing import List

import woodchips

Expand Down Expand Up @@ -26,3 +28,16 @@ def setup_logger():
def get_utc_timestamp():
"""Returns the UTC timestamp of right now."""
return datetime.datetime.now(datetime.timezone.utc)


def run_subprocess_command(command: List[str]) -> str:
"""Runs a shell command via subprocess."""
command_output = subprocess.check_output( # nosec
command,
stderr=subprocess.STDOUT,
text=True,
encoding='utf8',
timeout=Config.operation_timeout,
)

return command_output
2 changes: 2 additions & 0 deletions test/unit/test_deployments.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ def test_deploy_stage_success(mock_subprocess, mock_healthcheck, mock_path_exist
],
stderr=-2,
text=True,
encoding='utf8',
timeout=300,
)

Expand Down Expand Up @@ -209,5 +210,6 @@ def test_deploy_stage_prod_compose_success(mock_subprocess, mock_healthcheck, mo
],
stderr=-2,
text=True,
encoding='utf8',
timeout=300,
)
2 changes: 2 additions & 0 deletions test/unit/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def test_clone_repo(mock_subprocess, mock_logger, mock_project_path, mock_webhoo
['git', 'clone', '--depth=1', 'https://test-ssh-url.com', 'harvey/projects/test_user/test-repo-name'],
stderr=-2,
text=True,
encoding='utf8',
timeout=300,
)

Expand Down Expand Up @@ -63,6 +64,7 @@ def test_pull_repo(mock_subprocess, mock_logger, mock_project_path, mock_webhook
['git', '-C', 'harvey/projects/test_user/test-repo-name', 'pull', '--rebase'],
stderr=-2,
text=True,
encoding='utf8',
timeout=300,
)

Expand Down

0 comments on commit 95ca2a4

Please sign in to comment.