implemented ability to select python version #21
Conversation
chrisantonellis
commented
Feb 18, 2021
•
chrisantonellis
commented
- implemented ability to select python version via configuration, to determine which python version dependencies are installed for. previously system dependencies were installed for all available versions of python, but this resulted in error when testing with wheelhaus service.
- updated assoicated tests
…or via configuration
Codecov Report
@@ Coverage Diff @@
## main #21 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 7 7
Lines 189 199 +10
Branches 16 19 +3
=========================================
+ Hits 189 199 +10
|
| @@ -16,9 +16,13 @@ class Platform(str, Enum): | |||
| platform = PLATFORM | |||
There was a problem hiding this comment.
the tests were not accurately emulating the real constants, had to update the shape here
|
|
||
| __version__ = "0.1.0rc1" | ||
| __version__ = "0.1.0rc2" |
There was a problem hiding this comment.
prep for the immediate next RC release to test with wheelhaus service
little_cheesemonger/_run.py
Outdated
| if python_versions is None: | ||
| binaries_paths = list(get_python_binaries().values()) | ||
| else: | ||
| version_keys = [PythonVersion[version] for version in python_versions] |
There was a problem hiding this comment.
Since each version value is an external input, it might not be a valid key. The code should should validate the input or handle the exception.
There was a problem hiding this comment.
i'll add exception handling for now but ultimately i want to compare the configuration to a valid json schema or something that knows all the valid values for the platform 👍
little_cheesemonger/_run.py
Outdated
| """Install Python dependencies.""" | ||
|
|
||
| for binaries in get_python_binaries().values(): | ||
| if python_versions is None: | ||
| binaries_paths = list(get_python_binaries().values()) |
There was a problem hiding this comment.
It is not clear to me why values is being converted to a list since it is iterable.
There was a problem hiding this comment.
mypy was crying that the assignment was not compatible, List[ ] and ValuesIterable
There was a problem hiding this comment.
i left a comment 👍
There was a problem hiding this comment.
I think a different solution would have been to annotate the variable as Iterable[] to indicated that is the only interface you intend to use.
little_cheesemonger/_run.py
Outdated
| binaries_paths = list(get_python_binaries().values()) | ||
| else: | ||
| version_keys = [PythonVersion[version] for version in python_versions] | ||
| binaries_paths = [get_python_binaries()[version] for version in version_keys] |
There was a problem hiding this comment.
This will call get_python_binaries() N times. Instead, it could be called once before the if since the value is used in both blocks.
Co-authored-by: Floyd Hightower <[email protected]>
…fair-incubator/little-cheesemonger into cantonellis_implement_python_version
