search

growth-astro / growth-too-marshal

GROWTH Target of Opportunity Marshal
MIT License
13 stars 12 forks source link

add gattini to observation table #112

Closed mcoughlin closed 4 years ago

mcoughlin commented 4 years ago

Does this pull request make any changes to the database? No

Code changes that affect the database require special attention for data migration. If your change affects the database, describe the migration plan here.

mcoughlin commented 4 years ago

No I ran into issues when running the tests that it didn't know about the variables. How do you suggest getting around that?

On Nov 23, 2019 2:34 PM, "Leo Singer" notifications@github.com wrote:

@lpsinger requested changes on this pull request.

In growth/too/tasks/gattini_client.py https://github.com/growth-astro/growth-too-marshal/pull/112#discussion_r349893390 :

+ +@celery.task(base=PeriodicTask, shared=False, run_every=3600) +def gattini_obs(start_time=None, end_time=None): +

  • if start_time is None:
  • start_time = time.Time.now() - time.TimeDelta(1.0*u.day)
  • if end_time is None:
  • end_time = time.Time.now()
  • obstable = get_obs(start_time, end_time)
  • for ii, row in enumerate(obstable):
  • field_id, obsid, obstime, limmag = row
  • models.db.session.merge(
  • models.Observation(telescope='Gattini',
  • field_id=int(field_id),

What is the type of field_id? Is the cast necessary?

In growth/too/tasks/gattini_client.py https://github.com/growth-astro/growth-too-marshal/pull/112#discussion_r349893406 :

  • """, (start_time.jd, end_time.jd)).fetchall()
  • return result

  • +@celery.task(base=PeriodicTask, shared=False, run_every=3600) +def gattini_obs(start_time=None, end_time=None):

  • if start_time is None:
  • start_time = time.Time.now() - time.TimeDelta(1.0*u.day)
  • if end_time is None:
  • end_time = time.Time.now()
  • obstable = get_obs(start_time, end_time)
  • for ii, row in enumerate(obstable):

Remove unused enumeration variable.

In growth/too/flask.py https://github.com/growth-astro/growth-too-marshal/pull/112#discussion_r349893497 :

@@ -52,6 +51,14 @@ for dropin_file in dropin_files: app.config.from_pyfile(os.path.join('application.cfg.d', dropin_file))

+if "GATTINI_USER" in app.config:

It won't even try to connect to the database until you try to access the engine, so you need not check for the configuration variable to be present. Just provide a reasonable default value for the SQL bind URL, like we did in the past for the GROWTH marshal.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/growth-astro/growth-too-marshal/pull/112?email_source=notifications&email_token=AARPR27XISF6BJJQSXTDPTDQVGVXRA5CNFSM4JQYPMVKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCMYEWQI#pullrequestreview-321932097, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARPR23M2YIH5UVWFJZPEZLQVGVXRANCNFSM4JQYPMVA .

mcoughlin commented 4 years ago

And the ingestion failed without it, there is something weird about passing ints to sqlachemy which we ran into with ZTF as well.

On Nov 23, 2019 2:34 PM, "Leo Singer" notifications@github.com wrote:

@lpsinger requested changes on this pull request.

In growth/too/tasks/gattini_client.py https://github.com/growth-astro/growth-too-marshal/pull/112#discussion_r349893390 :

+ +@celery.task(base=PeriodicTask, shared=False, run_every=3600) +def gattini_obs(start_time=None, end_time=None): +

  • if start_time is None:
  • start_time = time.Time.now() - time.TimeDelta(1.0*u.day)
  • if end_time is None:
  • end_time = time.Time.now()
  • obstable = get_obs(start_time, end_time)
  • for ii, row in enumerate(obstable):
  • field_id, obsid, obstime, limmag = row
  • models.db.session.merge(
  • models.Observation(telescope='Gattini',
  • field_id=int(field_id),

What is the type of field_id? Is the cast necessary?

In growth/too/tasks/gattini_client.py https://github.com/growth-astro/growth-too-marshal/pull/112#discussion_r349893406 :

  • """, (start_time.jd, end_time.jd)).fetchall()
  • return result

  • +@celery.task(base=PeriodicTask, shared=False, run_every=3600) +def gattini_obs(start_time=None, end_time=None):

  • if start_time is None:
  • start_time = time.Time.now() - time.TimeDelta(1.0*u.day)
  • if end_time is None:
  • end_time = time.Time.now()
  • obstable = get_obs(start_time, end_time)
  • for ii, row in enumerate(obstable):

Remove unused enumeration variable.

In growth/too/flask.py https://github.com/growth-astro/growth-too-marshal/pull/112#discussion_r349893497 :

@@ -52,6 +51,14 @@ for dropin_file in dropin_files: app.config.from_pyfile(os.path.join('application.cfg.d', dropin_file))

+if "GATTINI_USER" in app.config:

It won't even try to connect to the database until you try to access the engine, so you need not check for the configuration variable to be present. Just provide a reasonable default value for the SQL bind URL, like we did in the past for the GROWTH marshal.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/growth-astro/growth-too-marshal/pull/112?email_source=notifications&email_token=AARPR27XISF6BJJQSXTDPTDQVGVXRA5CNFSM4JQYPMVKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCMYEWQI#pullrequestreview-321932097, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARPR23M2YIH5UVWFJZPEZLQVGVXRANCNFSM4JQYPMVA .