DM-28964: add incremental progress reporting to daf_butler #479
Conversation
TallJimbo
force-pushed
the
tickets/DM-28964
branch
from
22ac845
to
ad05b92
Compare
|
One really obvious flaw here is that the tests are failing, and it's because the global progress-handler state I've introduced interacts poorly with |
There was a problem hiding this comment.
The user interface looks fine. Given this is a "is this okay to continue" review request I'm not looking in detail. As for xdist, it forks a new python proces and is guaranteed to run setUpClass and setUp. It probably also runs setup_module. It's all meant to be distinct so I'm not sure what's going on.
Huh, that is at least not what I thought it did, so it will help me debug. It seems like it would have to be running other tests in the same process, between one |
|
I'm 99.9% sure that pytest guarantees that |
TallJimbo
force-pushed
the
tickets/DM-28964
branch
from
2a4234e
to
7a5bce3
Compare
TallJimbo
force-pushed
the
tickets/DM-28964
branch
6 times, most recently
from
346163d
to
68eba3b
Compare
| general the name of a group of progress bars, not necessarily as single | ||
| one, and it should have the same form as a logger names: the | ||
| fully-qualified name of the class, method, or function, minus the | ||
| initial "lsst" (and without any "implemntation detail" packages or |
There was a problem hiding this comment.
Minus the inital "lsst" if outside of daf_butler....
There was a problem hiding this comment.
I'll just remove the parenthetical.
| same name and level as ``self`` is enabled. | ||
| """ | ||
| if self._active_handler is not None: | ||
| logger = logging.getLogger(self._name) |
There was a problem hiding this comment.
Does this work for obs_base which uses lsst.log and not logging?
There was a problem hiding this comment.
I'm not sure if it works to set the level for a "logger" that really corresponds to a progress bar via lsst.log; I used logging to do that (while still using lsst.log for actual logs). I think that's okay, because I'm really just using the logging system as a way to get a global mapping of name->level, but it probably is worth documenting.
TallJimbo
force-pushed
the
tickets/DM-28964
branch
from
68eba3b
to
c8b8845
Compare
No low-level code has been implemented with this tooling yet, but once it is, high-level code will be able to produce progress bars and possibly other displays of how long various slow operations are taking. This does include an option to turn on progress via a Click-based progress bar in the butler command, but progress displays are off by default even there.
TallJimbo
force-pushed
the
tickets/DM-28964
branch
from
c8b8845
to
4aa88f8
Compare
This creates an abstract interface to allow at least daf_butler and some downstream middleware code to report progress in a way that would let higher-level tools show progress bars. It also makes a start at letting click-based command-line tools actually show those progress bars, but that's disabled by default as I'm not sure how it interacts with logging.
I haven't actually instrumented too many points in
daf_butler- just a few known hot spots, like datastore transfers and loops overexpandDataIdinRegistry- and there may not be much more we can do there, because we've actually done a pretty good job of vectorizing things, and that means a lot of slow operations are single commands from our perspective (and there's no way to e.g. get incremental progress on a query from the database). But if the middleware pundits are happy with the approach I've taken here, I'll go ahead and instrument at leastBaseSkyMap.register,RawIngestTask, andConvertRepoTask(on this ticket), where we certainly have lots of loops that would benefit from this kind of reporting.