Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AAP-40006 Empty docker directive on linting testenvs #695

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

BrennanPaciorek
Copy link
Contributor

linting testenvs do not need the database container

[AAP-40006] Do not create DB container on linting tox testenvs

Description

  • What is being changed? overriding default docker directive on linting targets with an empty one
  • Why is this change needed? Speed up lint targets
  • How does this change address the issue? Not creating and starting a database to run black, flake8, or isort testenvs, which do not use the database container

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Test update
  • Refactoring (no functional changes)
  • Development environment change
  • Configuration change

Self-Review Checklist

  • I have performed a self-review of my code
  • I have added relevant comments to complex code sections
  • I have updated documentation where needed
  • I have considered the security impact of these changes
  • I have considered performance implications
  • I have thought about error handling and edge cases
  • I have tested the changes in my local environment

Testing Instructions

Prerequisites

Fresh venv, if your working environment is old

Steps to Test

  1. run tox -m lint

Expected Results

docker should not run as a result of this command

@john-westcott-iv john-westcott-iv changed the title AAP-40006 Empty docker directive on linting testenvs [HOLD] AAP-40006 Empty docker directive on linting testenvs Feb 18, 2025
@john-westcott-iv
Copy link
Member

Adding [HOLD] until the related JIRA makes it into a sprint.

@BrennanPaciorek BrennanPaciorek changed the title [HOLD] AAP-40006 Empty docker directive on linting testenvs AAP-40006 Empty docker directive on linting testenvs Feb 25, 2025
@BrennanPaciorek
Copy link
Contributor Author

Un-[HOLD]'ing

@BrennanPaciorek BrennanPaciorek force-pushed the AAP-40006/fix/tox_docker_linting_targets branch from 1cf4539 to 73d24f8 Compare February 25, 2025 14:24
@BrennanPaciorek
Copy link
Contributor Author

BrennanPaciorek commented Feb 25, 2025

It seems these changes break tox -e <lint target> -- . not sure why though.

tox looks for a container . instead of treading . as a positional argument.

ValueError: Missing [docker:.] in tox.ini

linting testenvs do not need the database container
@BrennanPaciorek BrennanPaciorek force-pushed the AAP-40006/fix/tox_docker_linting_targets branch from 73d24f8 to 9115829 Compare February 25, 2025 17:05
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