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.

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

Add com.snowplowanalytics.iglu/client_error/jsonschema/1-0-0 #924

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chuwy
Copy link
Contributor

@chuwy chuwy commented Apr 9, 2019

No description provided.

@chuwy chuwy force-pushed the feature/iglu-client-error branch from cddfe79 to cdd9ce1 Compare April 10, 2019 11:00
@chuwy chuwy changed the title Add com.snowplowanalytics.iglu/client_error/jsonschema/1-0-0 (WIP) Add com.snowplowanalytics.iglu/client_error/jsonschema/1-0-0 Apr 10, 2019
@chuwy
Copy link
Contributor Author

chuwy commented Apr 10, 2019

Hey @BenFradet (maybe @yalisassoon as well), could you please review this one?

Couple of points:

  1. It represents either "resolution error" (schema is not available) or "validation error" (schema was found, but invalidated an instance)
  2. It probably won't be used anywhere directly as an instance will always be apart of bigger JSON (e.g. bad row), but it can be used as a reference of what Iglu Client is producing.
  3. I intentionally didn't include a schema key which caused an error because I believe this is information that user software (Enricher/Loader) owns and it is up to this software to include it properly. But obviously it must be included in a bad row
  4. Example of not-found error and example of data failed validation
  5. I'm very unhappy of three error/errors properties with different meanings and dataReports keyword. If you have better ideas - please let me know.

@chuwy chuwy requested a review from BenFradet April 10, 2019 11:08
@chuwy chuwy force-pushed the feature/iglu-client-error branch from cdd9ce1 to f293bef Compare April 10, 2019 11:10
Copy link
Contributor

@BenFradet BenFradet left a comment

Choose a reason for hiding this comment

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

Question: did we do away with Validation error - schema was found, but data is invalid or the schema itself is invalid from https://gist.github.com/chuwy/f03247b2c399d134d428aa81863bdfc4?

"version": "1-0-0"
},
"type": "object",
"required": ["error"],
Copy link
Contributor

Choose a reason for hiding this comment

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

does this relate to the anyOf?

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, some error is always present. Either ResolutionError or ValidationError

"type": "string"
},
"errors": {
"type": "array",
Copy link
Contributor

Choose a reason for hiding this comment

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

should there be a minItems?

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, good idea.

"minimum": 0
},
"fatal": {
"type": "boolean"
Copy link
Contributor

Choose a reason for hiding this comment

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

could we have a desc?

},
{
"description": "Data is invalid against its schema",
"properties": {
Copy link
Contributor

Choose a reason for hiding this comment

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

we seem to be missing required and additionalProperties

"minItems": 1,
"items": {
"type": "object",
"description": "List of reports",
Copy link
Contributor

Choose a reason for hiding this comment

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

this is just one report, no?

},
"targets": {
"description": "List of properties affected in instance",
"type": "array",
Copy link
Contributor

Choose a reason for hiding this comment

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

should there be a minItems?

@BenFradet BenFradet added this to the Bad rows milestone May 1, 2019
@chuwy
Copy link
Contributor Author

chuwy commented Dec 11, 2019

The schema is not used anywhere directly. But would be super-nice to have when we'll implement references. Descheduling.

@chuwy chuwy removed this from the Bad rows milestone Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants