Skip to content
This repository was archived by the owner on Mar 15, 2021. It is now read-only.

Comments

RFC0015: Schema resource#28

Merged
arnau merged 6 commits intomasterfrom
schema-api
Aug 22, 2018
Merged

RFC0015: Schema resource#28
arnau merged 6 commits intomasterfrom
schema-api

Conversation

@arnau
Copy link
Contributor

@arnau arnau commented Aug 17, 2018

Context

Before this RFC we explored a few options including JSON-LD, keep
something similar as the register register and field register or JSON
Schema. None of them is fit for purpose, mainly because our custom
datatype system.

Changes proposed in this pull request

This RFC proposes a schema resource to describe the register set of attributes
with their datatypes.

Guidance to review

It is consistent and forwards compatible.

Before this RFC we explored a few options including JSON-LD, keep
something similar as the register register and field register or JSON
Schema. None of them is fit for purpose, mainly because our custom
datatype system.

Signed-off-by: Arnau Siches <arnau.siches@digital.cabinet-office.gov.uk>
Arnau Siches added 4 commits August 17, 2018 17:09
Signed-off-by: Arnau Siches <arnau.siches@digital.cabinet-office.gov.uk>
Signed-off-by: Arnau Siches <arnau.siches@digital.cabinet-office.gov.uk>
Signed-off-by: Arnau Siches <arnau.siches@digital.cabinet-office.gov.uk>
Signed-off-by: Arnau Siches <arnau.siches@digital.cabinet-office.gov.uk>
@arnau arnau removed the wip label Aug 20, 2018
Copy link

@MatMoore MatMoore left a comment

Choose a reason for hiding this comment

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

I touched on this in #27, but could this be combined into the summary resource?

If a user cares about the schema they probably need the summary information as well, and keeping a small number of endpoints makes it easier to understand how to use the API.

@arnau arnau mentioned this pull request Aug 21, 2018
@arnau
Copy link
Contributor Author

arnau commented Aug 21, 2018

I touched on this in #27, but could this be combined into the summary resource?

It could but I made them different on purpose.

If a user cares about the schema they probably need the summary information as well, and keeping a small number of endpoints makes it easier to understand how to use the API.

I don't necessarily agree with "If a user cares about the schema they probably need the summary information as well" but it is probably true that part of the summary is likely to be used by a user that needs the schema.

I think the split makes more sense than keeping a single big multipurpose endpoint. The /schema endpoint is about letting people know how to process the data values. The summary is a mix of stats and general information. The latter might be something that shouldn't leave in the summary.

I'll think more about the split of responsibilities between the two endpoints.


|Name|Type|Description|
|-|-|-|
|`log-size` | Optional `Integer`| Set the log size to the given entry number instead of the latest value.|

Choose a reason for hiding this comment

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

Not sure I understand this param, is it to retrieve the schema at a particular point in the log?

If so, it would be great if the records endpoint could behave in the same way eventually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's the purpose and I agree the data counterpart should have the same

@arnau
Copy link
Contributor Author

arnau commented Aug 22, 2018

After @MatMoore comments here and in #27 and stepping back and thinking in context of #20 I'm making this RFC rejected because it's not fit for purpose.

Signed-off-by: Arnau Siches <arnau.siches@digital.cabinet-office.gov.uk>
@arnau arnau merged commit 6357928 into master Aug 22, 2018
arnau pushed a commit that referenced this pull request Aug 22, 2018
@arnau arnau deleted the schema-api branch August 22, 2018 09:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants