Refactored and deployed event schema stack #266
Conversation
* Finalised SequenceRunStateChange event schema - trim & case * Relocated event schemas directory into docs * Deployed schema stack Related #257
|
I will need to get merge this refactor PR first, before following up next for workflow event schema. At the same time, I need your input and review. So, will be waiting. |
There was a problem hiding this comment.
Hmmm, I just have the preference of having as close as possible to the code so perhaps stay as it is in the config folder 😅 (or maybe at the stack folder) ? I think the docs/ should be just a bunch of explanation on how to do what instead of relying some actual content.
Or it might be just me not used to it, so I'm happy either way 😁
config/constants.ts
Outdated
| @@ -76,7 +76,7 @@ export const rdsMasterSecretName = 'orcabus/master-rds'; // pragma: allowlist se | |||
| validateSecretName(rdsMasterSecretName); | |||
|
|
|||
| // Other constants that exist in the SharedStack | |||
| export const regName = 'OrcaBusSchemaRegistry'; | |||
| export const regName = 'orcabus.events'; | |||
There was a problem hiding this comment.
I don't think we have other registries planned, but can we be a bit more specific?
E.g. eventRegName or schemaRegName
There was a problem hiding this comment.
Ok, I will change it to schemaRegistryName
| "startTime", | ||
| "endTime", |
There was a problem hiding this comment.
You are keeping one record for each SequenceRun, right?
(i.e. there are defined start/end times and no intermediate state-bound times)
There was a problem hiding this comment.
Yes, that right. One record for each run; unique by instrumentRunId.
* Removed POC code comment from constants
Good point, Will. Schema is a bit tricky. Schema is a contract documentation by definition. Putting under docs section emphasizes this point. We might probably using some doc generator on these schemas to present them to user or observer, at some point down track. So, these folks won't need to dig through our code base, too much. |
|
Alexis; Ray; going to merge this now. Comment if any. |
Related #257