Fixed lockfile check when using a Docker read-only filesystem

[jfennick]

Fixes #

Changes made in this Pull Request:

• Added OSError in addition to PermissionError

PR Checklist

• [ ] Tests?
• [ ] Docs?
• [ ] CHANGELOG updated?
• [ ] Issue raised/referenced?

[codecov[bot]]

Codecov Report

Base: 94.32% // Head: 94.31% // Decreases project coverage by -0.00% :warning:

Coverage data is based on head (5b5ec9d) compared to base (463e52c). Patch coverage: 83.33% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3832      +/-   ##
===========================================
- Coverage    94.32%   94.31%   -0.01%     
===========================================
  Files          194      194              
  Lines        25062    25064       +2     
  Branches      3390     3391       +1     
===========================================
+ Hits         23639    23640       +1     
- Misses        1374     1375       +1     
  Partials        49       49              

Impacted Files	Coverage Δ
package/MDAnalysis/coordinates/XDR.py	99.29% <83.33%> (-0.71%)	:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[IAlibay]

Thanks @jfennick - since there are no related issues and it'll be hard to test the error, could you explain in more details what was the cause for this fix / any code/docker env details to duplicate the issue?

[jfennick]

Sure. Here is the relevant command line + stack trace from my log file.

/Users/jakefennick/workflow_inference_compiler/cachedir/fe47d299645b0f71eff2e8cbf791dcf9$ docker \
    run \
    -i \
    --mount=type=bind,source=/Users/jakefennick/workflow_inference_compiler/cachedir/fe47d299645b0f71eff2e8cbf791dcf9,target=/RvczZO \
    --mount=type=bind,source=/private/tmp/docker_tmpyrn_kcgl,target=/tmp \
    --mount=type=bind,source=/Users/jakefennick/workflow_inference_compiler/cachedir/1b588cb52801df5be9d783bad2d2cb63/npt.gro,target=/var/lib/cwl/stgdce8320f-73ef-4a3a-8db9-ea43afcfd074/npt.gro,readonly \
    --mount=type=bind,source=/Users/jakefennick/workflow_inference_compiler/cachedir/ae541201128048b4defdf1875c5dfa42/prod.trr,target=/var/lib/cwl/stg7d3f8150-98a4-4051-aed6-3f480f6cf22a/prod.trr,readonly \
    --workdir=/RvczZO \
    --read-only=true \
    --user=502:20 \
    --rm \
    --cidfile=/private/tmp/docker_tmp_n10lmap/20220916115735-218731.cid \
    --env=TMPDIR=/tmp \
    --env=HOME=/RvczZO \
    jakefennick/scripts \
    python3 \
    /align_protein_CA_mda.py \
    /var/lib/cwl/stgdce8320f-73ef-4a3a-8db9-ea43afcfd074/npt.gro \
    /var/lib/cwl/stg7d3f8150-98a4-4051-aed6-3f480f6cf22a/prod.trr \
    prod_align_protein_CA.trr
<Universe with 40893 atoms>
Traceback (most recent call last):
  File "/align_protein_CA_mda.py", line 15, in <module>
    trr.trajectory = TRR.TRRReader(input_trr_path) # This loads all frames.
  File "/miniconda/lib/python3.8/site-packages/MDAnalysis/lib/util.py", line 2495, in wrapper
    return func(self, *args, **kwargs)
  File "/miniconda/lib/python3.8/site-packages/MDAnalysis/coordinates/XDR.py", line 157, in __init__
    self._load_offsets()
  File "/miniconda/lib/python3.8/site-packages/MDAnalysis/coordinates/XDR.py", line 198, in _load_offsets
    with fasteners.InterProcessLock(lock_name) as filelock:
  File "/miniconda/lib/python3.8/site-packages/fasteners/process_lock.py", line 157, in __enter__
    gotten = self.acquire()
  File "/miniconda/lib/python3.8/site-packages/fasteners/process_lock.py", line 135, in acquire
    self._do_open()
  File "/miniconda/lib/python3.8/site-packages/fasteners/process_lock.py", line 107, in _do_open
    self.lockfile = open(self.path, 'a')
OSError: [Errno 30] Read-only file system: b'/var/lib/cwl/stg7d3f8150-98a4-4051-aed6-3f480f6cf22a/.prod.trr_offsets.lock'

For purposes of reproducibility/distribution with Common Workflow Language scripts, I am embedding align_protein_CA_mda.py within my Docker image jakefennick/scripts. My script works fine outside of Docker, but a Docker image is of course a read-only filesystem which causes MDAnalysis to raise an OSError, not a PermissionError.

For testing purposes, align_protein_CA_mda.py is rather trivial and should work with any standard gro and trr files.

[jfennick]

I guess I should clarify that cwltool automatically generates the above command and it includes the --read-only=true flag.

[jfennick]

Any movement on this issue?

[IAlibay]

Sorry about this @jfennick - I didn't get much headway on the alluded to blog post (fingers crossed end of this week). I'm dismissing my block, but could I ask you that you introduce yourself on the mailing list as per https://github.com/MDAnalysis/mdanalysis/pull/3832#pullrequestreview-1111137578 before we merge this?

We will need to trace contributors in the near future, so we need a way to reach out to everyone.

[jfennick]

Is there anything else I need to do to get this merged? If licensing issues are impeding drive-by contributions, then in the future perhaps I'll simply submit Issues instead of PRs.

[IAlibay]

@zemanj you have a blocking review here, could you have another look please?

[IAlibay]

Is there anything else I need to do to get this merged? If licensing issues are impeding drive-by contributions, then in the future perhaps I'll simply submit Issues instead of PRs.

Sorry about the long time to merge, the licensing stuff is not a blocker here (I dismissed my review some time back alongside https://github.com/MDAnalysis/mdanalysis/pull/3832#issuecomment-1266267258). I'll give @zemanj until next week to re-review otherwise I'll take over and approve.

[hmacdope]

@IAlibay just scrolling through and thought I would give this a bump.

[IAlibay]

Sorry about the last two commits, just thought it was faster for me to just add you to the changelog and fix the merge conflicts in one.

[jfennick]

Sorry about the last two commits, just thought it was faster for me to just add you to the changelog and fix the merge conflicts in one.

No problem. Whatever works.

[IAlibay]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[IAlibay]

CI failure is being solved in #3886 I'm going to go ahead with the squash merge to not hold things up.