#9739: Handle duplicate file basenames when testing multiple files in parallel
-----------------------------------------------+----------------------------
Reporter: mpatel | Owner: mvngu
Type: defect | Status: needs_review
Priority: critical | Milestone: sage-4.7.1
Component: doctest | Keywords: doctest
scripts
Work_issues: | Upstream: N/A
Reviewer: Robert Bradshaw, Leif Leonhardy | Author: Mitesh Patel,
John Palmieri
Merged: | Dependencies:
-----------------------------------------------+----------------------------
Changes (by leif):
* reviewer: Robert Bradshaw => Robert Bradshaw, Leif Leonhardy
Comment:
Replying to [comment:66 jhpalmieri]:
> True, theoretically, but I've never heard of this coming up. I don't
think these files are open for very long, so race conditions should be
very rare. I agree it should be dealt with on a follow-up, if at all.
(Instead of locking, we could perhaps use a "try ... except" block, since
if two processes are trying to write to the same timing file, it's not a
disaster if we just completely discard one of those.)
Ok, I thought timings were accumulated; never looked at this.
> I thought about this when working on the most recent patch, but it was
too much work for too little gain to make it backwards compatible (if
`.ptest_timing...` exists, then read it, otherwise, look for
`ptest_timing...`, etc.). It could be done on another ticket.
I wouldn't care much about backward compatibility in this case.
[[BR]]
> > > - the filename mangling uses the full path of the file rather than
`tempfile.mkstemp`; this should be good enough, especially since we work
in a directory with name determined by the pid and the hostname.
> >
> > This doesn't help when simultaneously testing the same file from one
`sage-ptest` instance (`SAGE_TEST_ITER`, `SAGE_TEST_GLOBAL_ITER`) if I'm
not wrong; we could mangle '''`sage-doctest`'s''' PID into the
''filename'', too, as I suggested above.
>
> I can do that, or we can use `mkstemp` instead of or in addition to the
path. Opinions either way?
I'd prefer `sage-doctest`'s PID, appended (separated by an underscore) as
[comment:62 above].
[[BR]]
> > Also, `sage-ptest` should know whether tests failed (or doctesting was
interrupted), in which case we don't have to issue a warning since keeping
the failed doctest files is intentional (and the left files should have
been mentioned in previous messages).
> > [[BR]]
> > So I wouldn't try to remove the directory if any doctests failed,
unless they were [all] interrupted by Ctrl-C (in which case `sage-doctest`
should immediately remove all temporary files belonging to the aborted
doctest, which it currently doesn't).
>
> Perhaps it should, but that should be on another ticket.
The deletion upon Ctrl-C in `sage-doctest`; I just wouldn't try to remove
the directory in `sage-ptest` if any error occurred (along with a perhaps
confusing warning message).
> "complete industrial strength bullet-proof solution"
I don't think we'll ever reach this, also because the addition of new
features will never stop. So Sage's version number won't converge.
> I can add a comment to the code about this.
I'd appreciate that, such that others can catch up. IMHO too much things
end up in ticket comments hardly anybody will see or read later.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9739#comment:67>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
