tickets/SP-1789: plot EFD logevents on an event timeline plot

[ehneilsen]

This PR is for the initial infrastructure and a few initial examples.
I expect plots to be supplemented and refined in additional issues and PRs.

[ehneilsen]

Times Square is down, so here is an example output plot.
example_timeline.html.gz

[rhiannonlynne]

I think it would be really useful to expand the documentation to make explicit

• why there are multiple sub-classes of the timeline log plot class (by making it clear what is different about the subclasses and WHY, it helps people understand what they would be expected to add if they make new subclasses .. you can see this to some extent from the code itself, but spelling it out is helpful especially if someone is coming in with a very different model of how it ought to work in mind)
• make it clear what the "kwarg" dict is supposed to be in the multitimeline plot function. Some examples in the docstring would be helpful.

I think that currently this is testing by running against the actual EFD. This is not super great -- Merlin suggested looking into "vcr" and at first glance, this seems like it might be useful. Can you look at replacing this unit test with one using vcr? I think that can be a separate ticket. Maybe disable the tests that run against the actual EFD unless the hostname is within usdf for now? (that's pretty close to "only run when triggered by a human" which is probably infrequent enough that it's fine).

[rhiannonlynne]

Here's the VCR readthedocs page - https://vcrpy.readthedocs.io/en/latest/
We use the EfdClient to do the efd queries, and I'm not 100% sure if this will confuse things or not, but eventually these are http queries so it might be just fine.

(one final thing -- I'm not entirely sold on swapping to makeEfdClient, mostly because it fails everywhere that's not a rubin site, and it's clear that the logging team are expecting to be able to run their logging tools outside those locations .... I'm a bit surprised this runs the tests in github with that import line in efd.py even?)

[ehneilsen]

I think that currently this is testing by running against the actual EFD. This is not super great -- Merlin suggested looking into "vcr" and at first glance, this seems like it might be useful. Can you look at replacing this unit test with one using vcr? I think that can be a separate ticket. Maybe disable the tests that run against the actual EFD unless the hostname is within usdf for now? (that's pretty close to "only run when triggered by a human" which is probably infrequent enough that it's fine).

Most of the test cases do not use the EDF, but rather construct the dictionary which would be returned by the EFD in the test case code itself. The exception is TestTimelinePlotters.test_example, which is skipped by default. Yes, it would be more thorough to write a full mock for the EFD client class.

[rhiannonlynne]

I think the thing with VCR is that you can skip most of the work, and just add the decorators then run the test somewhere where the HTTP responses can be recorded. Then later executions of the same test with the same parameters just use the recorded responses.
This does mean that changes in the EFD won't be available for tests, but that would be true of a mocked EFD anyway.

[ehneilsen]

(one final thing -- I'm not entirely sold on swapping to makeEfdClient, mostly because it fails everywhere that's not a rubin site, and it's clear that the logging team are expecting to be able to run their logging tools outside those locations .... I'm a bit surprised this runs the tests in github with that import line in efd.py even?)

I tried to be sure it would work if makeEfdClient wasn't available: see the try/except structure here. I suppose it would still fail if lsst.summit.utils.efdUtils is installed, but not at an Rubin site. I can fix that easily enough if I can figure out what exception that would throw.

[rhiannonlynne]

It's a RuntimeError https://github.com/lsst-sitcom/summit_utils/blob/b167aba8ce67140068bf61055731d4b1dd37044d/python/lsst/summit/utils/efdUtils.py#L471

Ok - yes, I see the try/except now .. I think I looked at the commit where it was added, and didn't see that it had changed by the end. Ok!