Skip to content
This repository was archived by the owner on Nov 20, 2025. It is now read-only.

fix: validate input schema using json schema validator - #44

Open
mhmd-azeez wants to merge 16 commits into
mainfrom
validate-input-schema
Open

fix: validate input schema using json schema validator#44
mhmd-azeez wants to merge 16 commits into
mainfrom
validate-input-schema

Conversation

@mhmd-azeez

@mhmd-azeez mhmd-azeez commented Jan 20, 2025

Copy link
Copy Markdown
Contributor

@mhmd-azeez
mhmd-azeez force-pushed the validate-input-schema branch from 77dc6dc to 281657b Compare January 21, 2025 08:46
@mhmd-azeez

Copy link
Copy Markdown
Contributor Author

not sure why the obsidian plugin is failing to build

@G4Vi

G4Vi commented Jan 21, 2025

Copy link
Copy Markdown
Contributor

not sure why the obsidian plugin is failing to build

There was a breaking change to the python-pdk. I will fix today.

Comment on lines 95 to 111
"type": "object",
"oneOf": []interface{}{
map[string]interface{}{
"required": []string{"audio_path"},
"not": map[string]interface{}{
"required": []string{"audio_base64"},
},
},
map[string]interface{}{
"required": []string{"audio_base64"},
"not": map[string]interface{}{
"required": []string{"audio_path"},
},
},
},
"properties": map[string]interface{}{
"audio_path": map[string]interface{}{

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The simulation can now handle complicated schemas like this too, @nilslice we just need to make sure the API/frontend can also handle it (when displaying it back on the website)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

awesome! i will try to use some of this code to help inform that update.

@mhmd-azeez
mhmd-azeez force-pushed the validate-input-schema branch from e533b55 to d565095 Compare January 22, 2025 16:06
@mhmd-azeez
mhmd-azeez force-pushed the validate-input-schema branch from 1688b98 to b8bbb37 Compare January 26, 2025 18:19
@mhmd-azeez

Copy link
Copy Markdown
Contributor Author

@nilslice @bhelx all servlets now pass the validation test. Please take a look and see if the validation rules are too restrictive

@bhelx bhelx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we merge? still valid?

@mhmd-azeez

Copy link
Copy Markdown
Contributor Author

If the business requirement is still the same, then the PR i think is still valid

Comment on lines +19 to +21
[[test]]
name = "describe output"
with = "../../simulations/describe-output/dist/plugin.wasm" No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what's the difference b/w using this local test and letting xtp handle the validation when pushing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was mainly using this to make sure all of the servlets are passing the validation (since this new version hasn't been pushed to xtp yet)

@bhelx

bhelx commented Apr 29, 2025

Copy link
Copy Markdown
Contributor

Checking to see if this is still valid. If not let's close it.

@mhmd-azeez

Copy link
Copy Markdown
Contributor Author

Fixed merge conflicts, @bhelx @nilslice If you're both okay with it, i will merge the PR upload the simulation

@bhelx

bhelx commented Jun 9, 2025

Copy link
Copy Markdown
Contributor

Checking in here to see what held us back from merging

@chrisdickinson
chrisdickinson removed their request for review June 27, 2025 17:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants