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.

Sign up for GitHub

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

Draft: SIP for JSONSchema version #24

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Draft: SIP for JSONSchema version #24

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b9c734
draft SIP for JSONSchema version
aaronsteers Oct 26, 2021
1018879
update latest version text
aaronsteers Nov 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions proposals/draft/Draft - JSONSchema Version Guidance.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
## Status

| header | header |
| ------ | ------ |
| State | Draft |
| Issue Link | https://github.com/MeltanoLabs/Singer-Working-Group/issues/17 |
| Discussion Thread(s) | (optional link) |
| Created | 2021-10-25 |

-----------------------

## Proposal

### TL;DR Overview

We should provide guidance for recommended and supported JSON Schema versions to tap and target developers which
Copy link

Choose a reason for hiding this comment

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

This sentence seems incomplete. Is there something missing from the overview statement?


### What specific change do you propose to make?

We should document in the Singer Spec that Singer taps and targets expect **Draft 04** version of the JSON Schema spec.
Copy link
Member

Choose a reason for hiding this comment

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

To expand a bit on this, we could even go as far as saying something like

Targets default to expecting Draft04 to be sent. To use a different json schema version use the $schema keyword. (To date nothing uses anything past Draft04 to my knowledge but I'm probably wrong based on how many taps/targets there are)

Taps should default to sending Draft04 unless they have a reason to use a later version of Json schema.

Copy link
Member

Choose a reason for hiding this comment

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

Should we also include a why here? ie Draft04 is widely used and works for the vast majority of use cases as of today.

Copy link

Choose a reason for hiding this comment

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

I like that statement in your first comment. No real need for a why in my opinion. The base Singer JSON Schema has been draft 4 and anything higher should be justified and proposed for its features. It seems okay to start there and move forward.


## Motivation

As of the time of writing, the latest version of json schema is **Draft 07**. While most changes between spec drafts are backwards compatible, we should nevertheless document for users and developer which version of the spec they should expect to use for best compatibility.
aaronsteers marked this conversation as resolved.
Show resolved Hide resolved

### Which layer(s) of the Singer ecosystem does this proposal directly touch?

Select all that apply:

- [x] Singer Specification - required capabilities and behaviors
- [ ] Singer Specification - optional capabilities and behaviors
- [ ] Singer best practices and other guidance
Copy link
Member

Choose a reason for hiding this comment

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

Is this a best practice or a required capability? Not clear to me

Copy link

Choose a reason for hiding this comment

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

After filling out the layered model proposal, I'd classify this as a standard I think (under that refactored model). Just because we all, as far as I know, have ascribed to it thus far. So I hope for opposition or sure acceptance of that statement :)

However, if this covers the use of the $schema keyword as per the previous comment. That would fall into a best practice under the layered proposal, which is non-required but recommended for targets to handle.

- [ ] Singer Working Group - practices and procedures
- [ ] Singer documentation (Other)

### What problem does it solve?

This prevents unnecessary trial and error and unnecessary uncertainty around which version(s) are supported or recommended.

### Why is it needed?

Without specific guidance, developers may waste development cycles unnecessarily (at best) or introduce incompatibilities (at worst).

## Other Considerations

### Are there any downsides to this change?

There are no downsides to this change.
Copy link

Choose a reason for hiding this comment

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

I would say there are always potential downsides. All specs have tradeoffs in my opinion. This one, I think, may be accepting a lack of verbosity for increased simplicity since Singer generally uses the JSON Schema spec for very basic data type specification and prefers to limit the usage of the more advanced value validation features.


### Is the change backwards compatible?

Not applicable.

### Which users are affected by the change?

Users are not affected.
Copy link

Choose a reason for hiding this comment

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

Users may be affected if they don't support Draft 4 at a minimum?


### How are users affected by the change? (e.g. DB upgrade required?)

Not applicable.

### Prototype Implementations

Not applicable.
Copy link

Choose a reason for hiding this comment

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

I'd like to see more information here. I mean, I know that every singer-io tap has been held to the Draft 4 specification because that is what we use at Stitch to my knowledge.


### Future Plans

Not applicable.

### Excluded Alternatives

Not applicable.
Copy link

Choose a reason for hiding this comment

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

Maybe good to include the reasoning for not proposing to just keep the latest spec here?


### Acknowledgements

Not applicable.

## What defines this SIP as "done"?

This SIP is published and related documentation for Singer Spec is also updated with this guidance.