DM-54017: Use Felis for loading the APDB schemas#57
Conversation
bsmartradio
force-pushed
the
tickets/DM-54017
branch
from
3d0ce27 to
3815d37
Compare
gpdf
left a comment
gpdf
left a comment
There was a problem hiding this comment.
In commit b04b4ce, "Update schema to v11.0 and swap to Felis", a new schema is combined on the same commit with a new schema. We would encourage decoupling these so that it's possible to validate the new avsc-generation using Felis first against an existing one, and then separately committing the result of the schema generation in a different PR.
I also have some comments on the units in the generated schema, which probably need to be fed back to changes in the upstream Felis, where I do see that the "d" units are missing on the attributes corresponding to the comments below.
| ] | ||
| }, | ||
| { | ||
| "doc": "Time when this record was generated, expressed as Modified Julian Date, International Atomic Time.", |
| "type": "double" | ||
| }, | ||
| { | ||
| "doc": "Time when this record was marked invalid, expressed as Modified Julian Date, International Atomic Time.", |
| "type": "long" | ||
| }, | ||
| { | ||
| "doc": "Processing time when validity of this diaObject starts, expressed as Modified Julian Date, International Atomic Time.", |
gpdf
left a comment
gpdf
left a comment
There was a problem hiding this comment.
In commit b04b4ce, "Update schema to v11.0 and swap to Felis", a new schema is combined on the same commit with a new schema. We would encourage decoupling these so that it's possible to validate the new avsc-generation using Felis first against an existing one, and then separately committing the result of the actual schema generation in a different PR.
I also have some comments on the units in the generated schema, which probably need to be fed back to changes in the upstream Felis YAML rather than being fixed on this ticket. I'll submit a bug ticket to Jira for that.
Unfortunately, we cannot separate the Felis change and the schema change. We use Felis in updatSchema.py, which builds from the current sdm_schemas. The schema change is needed because a change was made a while back to updating fields were non-nullable. It wasn't caught that the change would have also required a major schema update. This currently does not cause issues, but still is a needed change. Additionally, the missing units will require a change to sdm_schemas, as we build everything from there. I can open a branch on sdm_schemas to make this change, assuming it needs to be present in all the documentation. I'd just like to confirm that that is what you are asking before making the changes there. |
3 participants
No description provided.