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

docs: add dataset schema validation #1304

Merged
merged 31 commits into from
Dec 6, 2024
Merged

Conversation

katacek
Copy link
Member

@katacek katacek commented Nov 27, 2024

resolves #1295 : adding documentation for dataset validation plus updating related part in monitoring

@katacek katacek added the t-console Issues with this label are in the ownership of the console team. label Nov 27, 2024
@katacek katacek added this to the 103rd sprint - Console team milestone Nov 27, 2024
@katacek katacek self-assigned this Nov 27, 2024
@@ -118,7 +118,7 @@ The template above defines the configuration for the default dataset output view

The default behavior of the Output tab UI table is to display all fields from `transformation.fields` in the specified order. You can customize the display properties for specific formats or column labels if needed.

![Output tab UI](./images/output-schema-example.png)
![Output tab UI](../images/output-schema-example.png)
Copy link
Member Author

Choose a reason for hiding this comment

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

the image is used also on some other page so I have left it in the original folder and just change the path, but it can be done either way (change the folder and path for the othe page)

@@ -41,12 +41,22 @@ Currently, the monitoring option offers the following features:

### Alert configuration

When you set up an alert, you have two choices for how you want the metrics to be evaluated. And depending on your choices, the alerting system will behave differently:
When you set up an alert, you have four choices for how you want the metrics to be evaluated. And depending on your choices, the alerting system will behave differently:
Copy link
Member Author

Choose a reason for hiding this comment

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

as a part of the validation, new monitoring possibilities arised, I have added them here

@katacek katacek marked this pull request as ready for review November 27, 2024 14:16
@katacek katacek requested a review from TC-MO as a code owner November 27, 2024 14:16
@katacek katacek requested a review from gippy November 29, 2024 11:50
@gippy gippy requested review from netmilk and mtrunkat December 2, 2024 10:31
Copy link
Member

@gippy gippy left a comment

Choose a reason for hiding this comment

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

Small changes needed, but otherwise I think this is hopefully good enough for most users :)

@katacek katacek requested a review from gippy December 3, 2024 08:12

:::info

The schema defines a single item in the dataset. Be careful not to define the schema as an array, it always needs to be a schema of an object.
Copy link
Member

Choose a reason for hiding this comment

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

@gippy, does user get an error when this happens?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not actually 100% sure. I think you could theoretically set the top level type of the schema to array. Will test it tomorrow. If it's possible then we will try to add some check to build so that the creator cannot do it.

Copy link
Member

Choose a reason for hiding this comment

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

Y, let's throw an error as early as possible so ideally in the build. Later it's too late.


## Dataset validation

When you define a schema of your default dataset, the schema is then always used when you insert data into the dataset to perform validation (we use [AJV](https://ajv.js.org/)).
Copy link
Member

Choose a reason for hiding this comment

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

One thing to consider - do we want to mention we currently use AJV? Perhaps AJV contains some JSON schema extensions, and if we replace them, we could change the expected behavior. Or is this not the thing?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to mention it, because the creators can then look up the validation output and what it means. I do not think AJV contains any additions to JSON schema.


**If the data you attempt to store in the dataset is invalid** (meaning any of the items received by the API fails the validation), **the whole request is discarded** and the API will return a response with status code 400 and the following JSON response:

```json
Copy link
Member

Choose a reason for hiding this comment

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

Please update the API docs with this and link them to this documentation. It's important to have it there.

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch, thanks, api docs part updated, link added

Screenshot 2024-12-05 at 12 10 04 Screenshot 2024-12-05 at 12 09 47

Copy link
Member

@gippy gippy left a comment

Choose a reason for hiding this comment

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

Approving, but I agree with Mara's comments

katacek and others added 12 commits December 4, 2024 17:49
@katacek katacek requested review from mtrunkat and TC-MO December 5, 2024 11:36
Copy link
Contributor

@TC-MO TC-MO left a comment

Choose a reason for hiding this comment

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

Good to go, spotted two things

  1. Double occurence of separate
  2. Typo

katacek and others added 2 commits December 6, 2024 11:02
@katacek katacek merged commit 6873e01 into master Dec 6, 2024
8 checks passed
@katacek katacek deleted the docs/add-dataset-schema-validation branch December 6, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t-console Issues with this label are in the ownership of the console team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prepare documentation for dataset validation
4 participants