Creating RFC to query by metadata schema versions so that components are notified only of data that they are compatible with. #90

maniarathi · 2019-07-25T21:26:15Z

August 12: Last day for community review
August 23: Last day for oversight review
RFC approved during the 2019-08-23 Technical Architecture Team meeting.

During community review, reviewers

corrected some errors in sample ElasticSearch queries and improved the style of others
debated the style of whether to nest the major and minor versions of schema versions or not
fixed some links
discussed how to best notify teams of changes to the metadata schema
discussed the benefits of having schema branch and revision numbers in the metadata, this was deferred to different RFC in name of moving forward with an MVP

mweiden · 2019-07-25T21:28:46Z

@maniarathi I'm offering to shepherd here, if needed

rfcs/text/0000-query-by-metadata-schema-versions.md

hannes-ucsc

@maniarathi and I agreed that pinning the version would be optional and I think this should be explicitly stated here. Apologies if I missed it. I also think this got moved too quickly from the Google doc to the RFC stage. There is ongoing discussion in the doc.

rfcs/text/0000-query-by-metadata-schema-versions.md

hannes-ucsc · 2019-07-26T20:54:57Z

rfcs/text/0000-query-by-metadata-schema-versions.md

+We propose adding two new required fields to each list of schema properties:
+
+```
+"schema_major_version": {


What schema file exactly in would this addition happen in? The same schema that specifies "describedBy"?

Also, consider a nested approach:

"schema_version": { "major": 1, "minor": 2 }

This appears in the provenance schema. https://github.com/HumanCellAtlas/metadata-schema/blob/master/json_schema/system/provenance.json

What's the benefit of the nested approach versus non-nested?

I would agree that this is a more straight-forward representation.

The plan is that this will be in the provenance section of the doc.

I will have to run this by the metadata folks... @simonjupp what do you think?

I think it's fine to have schema version fields in provenance. We want to ultimately move describedBy there, as well.

I think the question was about the nested versus non-nested approach to specifying the schema version. Definitely will be in the provenance block and I made a note as well.

This is a nit for me and my approval does not depend on it. I think the nesting makes sense and should be easy to implement. It would prevent the proliferation of related schema_… properties in the provenance section, for example if we add the revision number in the future. If this nit is addressed, the example queries below would have to be adjusted (s/schema_/schema.).

Ok sounds good! Still working on getting verification. The specific part I am unsure about is whether having nested properties means that I would have to create an entire new schema for the schema_version property. From pretty much every other example I've looked at, it seems as though I would have to create an entirely new schema_version.json schema which I would rather not do.

Technically you can nest the schemas for nested objects but I don't know if that's against the MD teams's rules. Even if it were, adding a module (that's what they call schemas for nested objects) would allow it to be reused in other places. And it would not be hard to do. I'll file the MD PR if that helps.

rfcs/text/0000-query-by-metadata-schema-versions.md

diekhans

Please describe that these fields are added to system/provenance.json and the rationale being that it can be added automatically by ingest.

That plus hannes' changes and I will give this an enthusiastic thumbs up

diekhans · 2019-07-27T21:54:53Z

It would also be good to provide a common library to implement create the subscriptions, rather than have each component recode the same thing.

calvinnhieu

Update example queries to reflect intended use case, but otherwise, sounds great to me!

rfcs/text/0000-query-by-metadata-schema-versions.md

maniarathi · 2019-07-29T20:11:32Z

It would also be good to provide a common library to implement create the subscriptions, rather than have each component recode the same thing.

@diekhans This is definitely something I consider a "nice-to-have" rather than a feature to stick in the MVP :) I added a section to the end of the RFC on Future Work and included the automatic generation there.

rfcs/text/0000-query-by-metadata-schema-versions.md

calvinnhieu

Looks good!

maniarathi · 2019-07-30T18:18:14Z

@diekhans Modified to add the note about the schema versions being placed into the provenance block.

Many people are confused about usage here: * #91 (comment) * #90 (comment) * ... have observed others

rfcs/text/0000-query-by-metadata-schema-versions.md

malloryfreeberg

I have some outstanding questions about the schema_revision_number and schema_branch fields, but I trust everyone else to reach a decision after I leave. So, happy to approve!

rfcs/text/0000-query-by-metadata-schema-versions.md

hannes-ucsc · 2019-07-31T20:57:04Z

rfcs/text/0000-query-by-metadata-schema-versions.md

+We propose adding two new required fields to each list of schema properties:
+
+```
+"schema_major_version": {


This is a nit for me and my approval does not depend on it. I think the nesting makes sense and should be easy to implement. It would prevent the proliferation of related schema_… properties in the provenance section, for example if we add the revision number in the future. If this nit is addressed, the example queries below would have to be adjusted (s/schema_/schema.).

* Hint that the title name should be replaced Many people are confused about usage here: * #91 (comment) * #90 (comment) * ... have observed others * Update rfc-template.md * Update rfc-template.md

rfcs/text/0000-query-by-metadata-schema-versions.md

* Hint that the title name should be replaced Many people are confused about usage here: * HumanCellAtlas#91 (comment) * HumanCellAtlas#90 (comment) * ... have observed others * Update rfc-template.md * Update rfc-template.md

mweiden · 2019-08-23T23:04:14Z

RFC approved during the 2019-08-23 Technical Architecture Team meeting.

…are notified only of data that they are compatible with.

sampierson · 2019-08-26T12:48:06Z

rfcs/text/0011-query-by-metadata-schema-versions.md

+### User Stories
+
+*Share the [User Stories](https://www.mountaingoatsoftware.com/agile/user-stories) motivating this RFC.*
+* As a data wrangle, I would like to be able to push new data adhering to the latest metadata schema without having to


s/wrangle/wranger/

sampierson · 2019-08-26T15:06:21Z

rfcs/text/0011-query-by-metadata-schema-versions.md

+this document simply designs the minimum system required to unblock wranglers from submitting data and protecting
+downstream components (and therefore the entire DCP) from breaking due to incompatibility.
+
+Note: the subscription modifications will only exist in the production environment. Once the metadata schema integration


I don't believe this mechanism should only run in production.
Integration should eagerly surface incompatibilities, sure. Chaos is more acceptable there.
However, the purpose of staging is to replicate as closely as possible the production environment, and be our "smoke test" before production deployment. This means it should bias towards stability, and use, as closely as possible, all the mechanisms that production uses.

* Hint that the title name should be replaced Many people are confused about usage here: * HumanCellAtlas#91 (comment) * HumanCellAtlas#90 (comment) * ... have observed others * Update rfc-template.md * Update rfc-template.md

…are notified only of data that they are compatible with. (HumanCellAtlas#90) * Creating RFC to query by metadata schema versions so that components are notified only of data that they are compatible with. * Creating link now that the RFC is approved.

maniarathi added rfc-community-review Architecture labels Jul 25, 2019

maniarathi requested review from samanehsan, hannes-ucsc, justincc, malloryfreeberg, calvinnhieu, xbrianh, kislyuk and morrisonnorman July 25, 2019 21:26

mweiden reviewed Jul 26, 2019

View reviewed changes

rfcs/text/0000-query-by-metadata-schema-versions.md Outdated Show resolved Hide resolved

rfcs/text/0000-query-by-metadata-schema-versions.md Outdated Show resolved Hide resolved

hannes-ucsc requested changes Jul 26, 2019

View reviewed changes

maniarathi force-pushed the maniarathi-schema-version-query-rfc branch from 953ae53 to dce82f5 Compare July 26, 2019 21:56

diekhans reviewed Jul 27, 2019

View reviewed changes

justincc approved these changes Jul 29, 2019

View reviewed changes

calvinnhieu requested changes Jul 29, 2019

View reviewed changes

rfcs/text/0000-query-by-metadata-schema-versions.md Outdated Show resolved Hide resolved

rfcs/text/0000-query-by-metadata-schema-versions.md Outdated Show resolved Hide resolved

maniarathi force-pushed the maniarathi-schema-version-query-rfc branch 2 times, most recently from ae43b90 to e7124df Compare July 29, 2019 20:10

xbrianh reviewed Jul 29, 2019

View reviewed changes

rfcs/text/0000-query-by-metadata-schema-versions.md Outdated Show resolved Hide resolved

xbrianh approved these changes Jul 30, 2019

View reviewed changes

maniarathi force-pushed the maniarathi-schema-version-query-rfc branch from e7124df to e9abcdb Compare July 30, 2019 18:12

calvinnhieu approved these changes Jul 30, 2019

View reviewed changes

maniarathi removed the request for review from malloryfreeberg July 30, 2019 18:18

kislyuk approved these changes Jul 30, 2019

View reviewed changes

mweiden added a commit that referenced this pull request Jul 30, 2019

Hint that the title name should be replaced

5a39bf5

Many people are confused about usage here: * #91 (comment) * #90 (comment) * ... have observed others

mweiden mentioned this pull request Jul 30, 2019

Hint that the title name should be replaced #95

Merged

mweiden reviewed Jul 30, 2019

View reviewed changes

rfcs/text/0000-query-by-metadata-schema-versions.md Outdated Show resolved Hide resolved

mweiden reviewed Jul 30, 2019

View reviewed changes

rfcs/text/0000-query-by-metadata-schema-versions.md Outdated Show resolved Hide resolved

malloryfreeberg approved these changes Jul 31, 2019

View reviewed changes

samanehsan approved these changes Jul 31, 2019

View reviewed changes

hannes-ucsc requested changes Jul 31, 2019

View reviewed changes

maniarathi force-pushed the maniarathi-schema-version-query-rfc branch from 47793da to a3935ad Compare August 1, 2019 14:31

hannes-ucsc requested changes Aug 1, 2019

View reviewed changes

rfcs/text/0000-query-by-metadata-schema-versions.md Outdated Show resolved Hide resolved

rfcs/text/0000-query-by-metadata-schema-versions.md Outdated Show resolved Hide resolved

maniarathi force-pushed the maniarathi-schema-version-query-rfc branch from 70a6471 to 664ac9b Compare August 2, 2019 22:02

hannes-ucsc approved these changes Aug 3, 2019

View reviewed changes

This was referenced Aug 6, 2019

Create documentation on how to specify schema versions in a subscription query HumanCellAtlas/dcp#436

Closed

Populate schema version information in provenance block HumanCellAtlas/ingest-client#44

Closed

mweiden added rfc-oversight-review and removed rfc-community-review labels Aug 13, 2019

morrisonnorman approved these changes Aug 16, 2019

View reviewed changes

mweiden added rfc-approved and removed rfc-oversight-review labels Aug 23, 2019

Creating RFC to query by metadata schema versions so that components …

66c9bf6

…are notified only of data that they are compatible with.

maniarathi force-pushed the maniarathi-schema-version-query-rfc branch from 664ac9b to ba59206 Compare August 23, 2019 23:17

Creating link now that the RFC is approved.

38d6618

maniarathi force-pushed the maniarathi-schema-version-query-rfc branch from ba59206 to 38d6618 Compare August 23, 2019 23:19

maniarathi merged commit 52c0b55 into master Aug 23, 2019

maniarathi deleted the maniarathi-schema-version-query-rfc branch August 23, 2019 23:19

sampierson reviewed Aug 26, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Creating RFC to query by metadata schema versions so that components are notified only of data that they are compatible with. #90

Creating RFC to query by metadata schema versions so that components are notified only of data that they are compatible with. #90

maniarathi commented Jul 25, 2019 •

edited by mweiden

mweiden commented Jul 25, 2019

hannes-ucsc left a comment

hannes-ucsc Jul 26, 2019

maniarathi Jul 26, 2019

diekhans Jul 27, 2019

justincc Jul 29, 2019

maniarathi Jul 29, 2019

malloryfreeberg Jul 31, 2019

maniarathi Jul 31, 2019

hannes-ucsc Jul 31, 2019

maniarathi Aug 2, 2019

hannes-ucsc Aug 3, 2019

diekhans left a comment

diekhans commented Jul 27, 2019

calvinnhieu left a comment

maniarathi commented Jul 29, 2019

calvinnhieu left a comment

maniarathi commented Jul 30, 2019

malloryfreeberg left a comment

hannes-ucsc Jul 31, 2019

mweiden commented Aug 23, 2019

sampierson Aug 26, 2019

sampierson Aug 26, 2019

hannes-ucsc Aug 27, 2019

Creating RFC to query by metadata schema versions so that components are notified only of data that they are compatible with. #90

Creating RFC to query by metadata schema versions so that components are notified only of data that they are compatible with. #90

Conversation

maniarathi commented Jul 25, 2019 • edited by mweiden

mweiden commented Jul 25, 2019

hannes-ucsc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diekhans left a comment

Choose a reason for hiding this comment

diekhans commented Jul 27, 2019

calvinnhieu left a comment

Choose a reason for hiding this comment

maniarathi commented Jul 29, 2019

calvinnhieu left a comment

Choose a reason for hiding this comment

maniarathi commented Jul 30, 2019

malloryfreeberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mweiden commented Aug 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maniarathi commented Jul 25, 2019 •

edited by mweiden