sot / kadi

Chandra commands and events
https://sot.github.io/kadi
BSD 3-Clause "New" or "Revised" License
5 stars 3 forks source link

Web-free kadi #231

Closed taldcroft closed 2 years ago

taldcroft commented 2 years ago

Description

This removes the web server part of the kadi package, instead making kadi events be strictly a standalone django ORM package.

Importantly, this removes the django.setup() from the kadi.events init and puts it into query.py. At the same time the symbols from query.py are only lazily imported on demand. This allows two things to work:

The one downside of the setup reorganization is that the list of available event queries needs to be hard-coded in kadi/events/__init__.py. I could not find a way around this, but given the stability of the event types this is not a problem in practice.

To do:

Interface impacts

None. This still needs testing but that is the intent.

Testing

Unit tests

Independent check of unit tests by [REVIEWER NAME]

Functional tests

Test update_events.py script

Rebuilt the events3.db3 database following the steps in NOTES.build.

cd ~/git/kadi
export KADI=$PWD
rm -f events3.db3
rm -rf kadi/events/migrations
./manage.py makemigrations events
./manage.py migrate

python -m kadi.scripts.update_events --start=1999:001 --stop=2000:001
python -m kadi.scripts.update_events --start=2000:001

Module lazy imports

In [1]: from kadi import events

In [2]: dir(events)
Out[2]: 
['caps',
 'dark_cal_replicas',
 'dark_cals',
 'dsn_comms',
 'dumps',
 'dwells',
 'eclipses',
 'fa_moves',
 'grating_moves',
 'load_segments',
 'ltt_bads',
 'major_events',
 'manvrs',
 'models',
 'normal_suns',
 'obsids',
 'orbits',
 'pass_plans',
 'rad_zones',
 'safe_suns',
 'scs107s',
 'tsc_moves']

In [3]: events.<TAB>  # Works as expected and shows the above list

In [4]: from kadi.events import *

In [5]: whos
Variable            Type                Data/Info
-------------------------------------------------
caps                EventQuery          <EventQuery: CAP>
dark_cal_replicas   EventQuery          <EventQuery: DarkCalReplica>
dark_cals           EventQuery          <EventQuery: DarkCal>
dsn_comms           EventQuery          <EventQuery: DsnComm>
dumps               EventQuery          <EventQuery: Dump>
dwells              EventQuery          <EventQuery: Dwell>
eclipses            EventQuery          <EventQuery: Eclipse>
events              module              <module 'kadi.events' fro<...>kadi/events/__init__.py'>
fa_moves            EventQuery          <EventQuery: FaMove>
grating_moves       EventQuery          <EventQuery: GratingMove>
load_segments       EventQuery          <EventQuery: LoadSegment>
ltt_bads            LttBadEventQuery    <EventQuery: LttBad>
major_events        EventQuery          <EventQuery: MajorEvent>
manvrs              EventQuery          <EventQuery: Manvr>
models              module              <module 'kadi.events.mode<...>i/kadi/events/models.py'>
normal_suns         EventQuery          <EventQuery: NormalSun>
obsids              EventQuery          <EventQuery: Obsid>
orbits              EventQuery          <EventQuery: Orbit>
pass_plans          EventQuery          <EventQuery: PassPlan>
rad_zones           EventQuery          <EventQuery: RadZone>
safe_suns           EventQuery          <EventQuery: SafeSun>
scs107s             EventQuery          <EventQuery: Scs107>
tsc_moves           EventQuery          <EventQuery: TscMove>
taldcroft commented 2 years ago

@javierggt - can you give this a spin in web-kadi-test and see if it fixes the intermittent 500 errors?

javierggt commented 2 years ago

This branch is now installed in the web-kadi-test environment, so it is running on kadi-test. The usual error has not shown up yet. We should just start using it more regularly to see.

taldcroft commented 2 years ago

I just started a script to just pound on the kadi events API. After 5 minutes it is still going with no errors.

import sys
import time
import numpy as np
import requests
from cxotime import CxoTime

queries = [
    'obsids',
    'tsc_moves',
    'dark_cal_replicas',
    'dark_cals',
    'scs107s',
    'fa_moves',
    'grating_moves',
    'dumps',
    'eclipses',
    'manvrs',
    'dwells',
    'safe_suns',
    'normal_suns',
    'major_events',
    'caps',
    'load_segments',
    'pass_plans',
    'dsn_comms',
    'orbits',
    'rad_zones',
    'ltt_bads'
]

n_req = 0
while True:
    start = CxoTime('2021:001', out_subfmt='date') + np.random.randint(0, 300)
    stop = start + 3
    stop.out_subfmt = 'date'
    query = np.random.choice(queries)
    url = f'https://web-kadi-test.cfa.harvard.edu/api/kadi/events/{query}/filter?start={start}&stop={stop}'
    t0 = time.time()
    rq = requests.get(url)
    n_req += 1
    t1 = time.time()
    if rq.status_code != 200:
        print()
        print('*' * 30, 'Bad request', '*' * 30)
        print(rq.status_code)
        print(rq.text)

    try:
        rq.json()
        print(query[0], end='')
        sys.stdout.flush()
    except Exception as exc:
        print()
        print('*' * 30, 'Bad JSON', '*' * 30)
        print(exc)
taldcroft commented 2 years ago

I made 5837 requests with no errors. If you agree the script is providing coverage of the issue then I think this looks pretty good.

There is still the problem of importing kadi.events.models directly without query first. In practice I think this is not a problem and we could consider the models module to be private. But I agree this is a bit unorthodox.

javierggt commented 2 years ago

I think that looks good. Most probably the error was because the way I made my local change.

We could catch the AppRegistryNotReady at the top of kadi.events.models and issue a more informative error.

I checked, and the following seem to import kadi.events.models:

Another option is to catch AppRegistryNotReady and call django.setup() in kadi.events.models

taldcroft commented 2 years ago

I checked, and the following seem to import kadi.events.models

Great catch. I had at one point tested that the update_events script works, but it was indeed broken with an AppRegistryNotReady exception. But (finally something that works), I discovered a minimal workaround that seems to do the trick. See 1fba5d9 and the very long comment there.

I'm running the steps in NOTES.build now to rebuild the entire db from scratch and so far all looks good.

javierggt commented 2 years ago

But (finally something that works), I discovered a minimal workaround that seems to do the trick. See https://github.com/sot/kadi/commit/1fba5d93f40034e11d5df4f4579f909377a99489 and the very long comment there.

Unfortunately, I am not sure I understand the comment there (and I have followed our discussion!).

The phrase "we need A to run B" can be understood as "we need that A runs B" (as in "models should call setup") or "to run B, we need A" (as in "to import models we need to call setup first"). Very similar but it confuses me. There is a typo in line 63. I didn't understand exactly what the trick was.