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

Error docker.execute with str #501

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

kashyab12
Copy link
Contributor

@kashyab12 kashyab12 commented Nov 14, 2023

#463

Getting rid of str annotations from command variables. I did notice that the execute method for the ContainerCLI class explicitly asserts for the command var to be of list type and throws an appropriate exception message if it is not. I decided not to perform the same assertion within the ComposeCLI class's execute function and the ServiceCLI class's create function since beartype can be of value of here (see comments within #466) . However, I am open to feedback regarding this.

@gabrieldemarmiesse
Copy link
Owner

Thank you for the pull request. Indeed, I'm still thinking about beartype from time to time, weighting the pros and cons (we might loose duck typing unless we use more complex types, like protocols).

Anyways, your PR is a strict improvement compared to the previous behaviour, so let's merge it

@gabrieldemarmiesse gabrieldemarmiesse merged commit a46a133 into gabrieldemarmiesse:master Nov 15, 2023
36 checks passed
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