Photometry passthrough#582
Conversation
thomasculino
left a comment
thomasculino
left a comment
There was a problem hiding this comment.
It looks pretty good. I just have some questions
| [submodule "skyportal"] | ||
| path = skyportal | ||
| url = https://github.com/skyportal/skyportal.git | ||
| url = https://github.com/mcoughlin/skyportal.git |
There was a problem hiding this comment.
We should add the appropriate changes to skyportal main then revert that change before merging. This way we avoid losing edits we made if we pin skyportal to a new commit later.
| survey2instrumentid = make_survey2instrumentid(session) | ||
| programid2streamid = make_programid2stream_mapper(session) | ||
|
|
||
| # Same query the persisting POST path uses: pull the latest alert and join in |
There was a problem hiding this comment.
We're actually pulling the brightest alert but it does't make a big difference.
There was a problem hiding this comment.
If there's new photometry on boom after the cache is filled, it won't show up until the cache expires or ?refresh=true is used. Since there's no expiry time, the cached data could be pretty out of date. Wouldn't it be worth adding a cache expiry so the broker points are periodically re-fetched, or getting rid of the cache for this object whenever new photometry is persisted?
| def groups_to_points(groups): | ||
| """Flatten per-(survey, programid) groups into a flat list of photometry | ||
| points. Each point carries the transform's SkyPortal-unit fields plus the | ||
| ``instrument_id``; ``stream_ids`` are intentionally omitted from the payload | ||
| (they gate visibility but are not display data). | ||
| """ |
There was a problem hiding this comment.
Not sure if this is the right place for this function. It's used only in the tests, should we move it?
There was a problem hiding this comment.
Is there a PR opened on skyportal for this? I don't want us to lose anything if we pin skyportal to another commit later (same worry as my previous comment).
|
@thomasculino skyportal/skyportal#6153 the SkyPortal PR is there. If it makes sense, we could also move some stuff to SkyPortal too. |
Ah nice! I didn't see it. I don't know what could get moved to Skyportal here, it looks very Fritz specific. Once the PRs on Skyportal are merged, I'll merge this one |
2 participants
This PR makes photometry passthrough (with caching) possible.