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

VDB Matrix: Vendor Data #44

Merged
merged 15 commits into from
Jan 10, 2024
Merged

VDB Matrix: Vendor Data #44

merged 15 commits into from
Jan 10, 2024

Conversation

svonava
Copy link
Contributor

@svonava svonava commented Jan 4, 2024

Context:

This PR contains:

  1. JSON schema that defines the vendor data we want to capture.
  2. A script that converts the original spreadsheet to this new data structure (will be ran once in the moment of the switch over from the original spreadsheet to the vectorhub-powered table).
  3. The actual vendor-specific JSON files.

The front-end and github->front-end sync parts will be delivered in separate PRs in parallel to this one.

Copy link
Collaborator

@mamayer19 mamayer19 left a comment

Choose a reason for hiding this comment

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

If you want to use the new schema for the github action, that will have to be modified as well. It's currently using docs/tools/vdb_table/data/vendor.schema.json and not docs/tools/vdb_table/vendor.schema.json

@svonava
Copy link
Contributor Author

svonava commented Jan 4, 2024

@mamayer19 thanks that helped. However I introduced a regression:
https://github.com/superlinked/VectorHub/actions/runs/7410206699/job/20161978303?pr=44

I saw this trigger locally also, but can't tell why stringWithSource wouldn't succeed validation - it's declared the same way as everything else in that schema.

@mamayer19
Copy link
Collaborator

mamayer19 commented Jan 4, 2024

@mamayer19 thanks that helped. However I introduced a regression: https://github.com/superlinked/VectorHub/actions/runs/7410206699/job/20161978303?pr=44

I saw this trigger locally also, but can't tell why stringWithSource wouldn't succeed validation - it's declared the same way as everything else in that schema.

I think you are referencing these wrong. It should work with:

"license": {"allOf": [{"$ref": "#/$defs/string"}], "$comment": "About | License | The license the source code is released under." },

My bet is that none of the others work either, this is the first it hits and it exits the validation. There's only links before it, where the name of the def and the id are the same, so it validates fine. I don't think you $ref the $id. You $ref the $defs name.

@mamayer19
Copy link
Collaborator

I see the fix worked for the validation, so I only have 1 remaining suggestion: I highly advise against using spaces in file names, I'd rather convert everything to snake case. Spaces cause notoriously annoying issues all the time on *nix-like systems.

@svonava
Copy link
Contributor Author

svonava commented Jan 4, 2024

@mamayer19 nice good point, fixed

@svonava svonava merged commit 693b2a3 into superlinked:main Jan 10, 2024
1 check passed
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.

3 participants