datalad / datalad-container

DataLad extension for containerized environments
http://datalad.org
Other
11 stars 17 forks source link

Support `{python}` placeholder #227

Closed mih closed 1 year ago

mih commented 1 year ago

This placeholder is expanded on execution of a container, rather than on configuration/addition. This helps use the same Python installation that is also executing the datalad-container code.

Previously, the docker-support code would expand and then hardcode sys.executable on configuring a container. This led to non-portable configuration (e.g., hardcoded python.exe on windows), and would fail to pick up the correct python installation in any case where the python entrypoint would not point to the correct one.

Closes #226 Almost does #224 (missing the platform path issue)

codeclimate[bot] commented 1 year ago

Code Climate has analyzed commit 3fc00796 and detected 0 issues on this pull request.

View more on Code Climate.

adswa commented 1 year ago

The failures are due to dockerhubs rate limiting:

toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit
mih commented 1 year ago

@adswa Yeah, the test setup is not so nice. Locally this works now -- also on windows.

yarikoptic commented 1 year ago

any idea on how many tests would fail if windows is added as a test env?

mih commented 1 year ago

any idea on how many tests would fail if windows is added as a test env?

When I execute the tests on windows (minus anything metalad-related https://github.com/datalad/datalad-metalad/issues/397 because I am working with PY3.12 right now), I see 8 failed, 13 passed.

A good chunk of the failures is simply https://github.com/datalad/datalad-container/issues/233, most of the rest is wrong path assumptions.

mih commented 1 year ago

Rebased after merge of #235

codecov[bot] commented 1 year ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (4782a7a) 94.59% compared to head (3fc0079) 94.59%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #227 +/- ## ======================================= Coverage 94.59% 94.59% ======================================= Files 25 25 Lines 1091 1092 +1 ======================================= + Hits 1032 1033 +1 Misses 59 59 ``` | [Files](https://app.codecov.io/gh/datalad/datalad-container/pull/227?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=datalad) | Coverage Δ | | |---|---|---| | [datalad\_container/containers\_add.py](https://app.codecov.io/gh/datalad/datalad-container/pull/227?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=datalad#diff-ZGF0YWxhZF9jb250YWluZXIvY29udGFpbmVyc19hZGQucHk=) | `89.67% <100.00%> (ø)` | | | [datalad\_container/containers\_run.py](https://app.codecov.io/gh/datalad/datalad-container/pull/227?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=datalad#diff-ZGF0YWxhZF9jb250YWluZXIvY29udGFpbmVyc19ydW4ucHk=) | `84.84% <100.00%> (+0.23%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mih commented 1 year ago

Merging, given the approvals.