Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-34480: Add APDB schema definition based on felis #52

Merged
merged 3 commits into from May 13, 2022

Conversation

andy-slac
Copy link
Contributor

This schema is the same as currently defined in existing YAML files in
dax_apdb and ap_association packages. Some columns have their nullable
attributes corrected to reflect the design. There are differences w.r.t.
baselineSchema, in particular APDB schema does not include few columns
that were added recently to baseline schema - nearbyExtObj[1-3],
nearbyLowzGal, and corresponding separation columns. And there are few
columns in APDB that do not exist in baseline schema for DIA tables.

yml/apdb.yaml Outdated
# datatype: float
# fits:tunit: arcsec
# description: Separation distance of nearbyLowzGal.
# mysql:datatype: FLOAT
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bunch of columns are commented out, these were added recently to baselineSchema. If they are not useful in APDB I can remove them, otherwise I can un-comment them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not implemented in AP yet but will be, so please do include them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have uncommented these new columns.

Another small change since review - length attribute added to timestamp and binary columns. length is not used currently by APDB classes, it is mostly to suppress felis messages.

Copy link
Contributor

@ebellm ebellm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of schema changes.

yml/apdb.yaml Outdated
# datatype: float
# fits:tunit: arcsec
# description: Separation distance of nearbyLowzGal.
# mysql:datatype: FLOAT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not implemented in AP yet but will be, so please do include them.

- "#DiaObject.validityStart"
mysql:engine: MyISAM
mysql:charset: latin1
- name: SSObject
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These SSObject tables are out of date. The correct ones are at https://jira.lsstcorp.org/browse/RFC-620 (already in felis YAML!)

We might as well update baselineSchema.yaml at the same time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SSObject schema was replaced with the one from RFC-620, in both new apdb.yaml and baselineSchema.yaml.

This schema is the same as currently defined in existing YAML files in
dax_apdb and ap_association packages. Some columns have their `nullable`
attributes corrected to reflect the design. There are differences w.r.t.
baselineSchema, in particular APDB schema does not include few columns
that were added recently to baseline schema - `nearbyExtObj[1-3]`,
`nearbyLowzGal`, and corresponding separation columns. And there are few
columns in APDB that do not exist in baseline schema for DIA tables.
Copy link
Contributor

@ebellm ebellm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @andy-slac--apologies for not being clear--you should include all of the tables defined in RFC-620 in the apdb and baseline schemas (MPCORB, MPCORBDESIGMAP, SSObject, SSSource). Thanks!

Both `apdb.yaml` and `baselineSchema.yaml` not have SS Products tables
schema as defined in RFC-620.
@andy-slac
Copy link
Contributor Author

Hi @andy-slac--apologies for not being clear--you should include all of the tables defined in RFC-620 in the apdb and baseline schemas (MPCORB, MPCORBDESIGMAP, SSObject, SSSource). Thanks!

@ebellm, sorry, I misunderstood that. I have force-pushed new version with all four tables from RFC-620. I ran all tests again, and restarted Jenkins.

@andy-slac andy-slac requested a review from ebellm May 12, 2022 23:57
@andy-slac andy-slac merged commit 02bbe54 into main May 13, 2022
@andy-slac andy-slac deleted the tickets/DM-34480 branch May 13, 2022 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants