-
Notifications
You must be signed in to change notification settings - Fork 8
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
Feature/fhir json schema + Remove Elasticsearch support from Grip #318
Conversation
func (server *GripServer) LoadSchemas(project_id string, sch *gripql.Graph, out *graph.GraphSchema) (*graph.GraphSchema, error) { | ||
schcompiler := jsonschema.NewCompiler() | ||
schcompiler.ExtractAnnotations = true | ||
schcompiler.RegisterExtension(compile.GraphExtensionTag, compile.GraphExtMeta, compile.GraphExtCompiler{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our graph extension is hard coded into the loader here. An extension argument could get abstracted out as an argument to the load schema protobuf method.
@@ -171,7 +173,7 @@ var Cmd = &cobra.Command{ | |||
|
|||
if yamlFile != "" { | |||
log.Infof("Loading YAML file: %s", yamlFile) | |||
graphs, err := gripql.ParseYAMLGraphsFile(yamlFile) | |||
graphs, err := schema.ParseYAMLGraphsFile(yamlFile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why isn't this graphSchema?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
docs/categories/index.xml
Outdated
@@ -2,13 +2,10 @@ | |||
<rss version="2.0" xmlns:atom="http://www.w3.org/2005/Atom"> | |||
<channel> | |||
<title>Categories on GRIP</title> | |||
<link>https://bmeg.github.io/grip/categories/</link> | |||
<link>http://localhost:1313/grip/categories/</link> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doc build was done with the wrong hostname
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed in newest commit
ok localhost paths should be fixed now. When |
…aphql conversion scripts into jsonschemagraph package
Moved out graphql code. This branch should be cleaned up now and ready for merge |
|
||
def test_post_json_schema(man): | ||
errors = [] | ||
G = man.setGraph("swapi") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For cases where the unit test is a write option, you get the graph using
G = man.writeTest()
conformance/tests/ot_schema.py
Outdated
errors = [] | ||
G = man.setGraph("swapi") | ||
# Probably don't want to add a 2MB schema file to the repo so get it via requests instead | ||
res = requests.get("https://raw.githubusercontent.com/bmeg/iceberg/f1724941fe47df24846135fb515d1b89e791cee3/schemas/graph/graph-fhir.json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably isn't ideal in the long run. We can think about a paired down sample schema that can be used for unit tests in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Is there a way to load a directory of JsonSchema YAML files? I only see single file JSON post for the |
cmd/schema/main.go
Outdated
sflags.BoolVar(&manual, "manual", manual, "Use client side schema sampling") | ||
sflags.StringSliceVar(&excludeLabels, "exclude-label", excludeLabels, "exclude vertex/edge label from schema") | ||
pflags.StringVar(&jsonSchemaFile, "jsonSchema", "", "Json Schema") | ||
pflags.StringVar(&graphName, "graphName", "", "Name of schemaGraph") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the other sub commands, graphName is defined via fixed argument and not a flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
This new feature has been added see tests, cmd line option --yamlSchemaPath supports loading individual yaml schema as files or loading entire yaml schema directories of files. Additionally the AddSchema API has been changed from a delete and replace strategy to a upsert strategy so that schemas can now be added to / edited with multiple AddSchema calls. |
Should be ready for another review / merge now |
LGTM |
This PR does a few things:
Add support for storing jsonschema as a grip graph
grip schema post --graphName CALIPER --jsonSchema test.json
The protobuf for this is defined as:
Add support for loading FHIR json directly into grip via new proto method:
grip caliperload schema-test/Observation.ndjson CALIPER test-data2
Note also BulkJsonEditResult is also added, adding support for returning messages back to the client so that an HTTP code can be generated from the error messages or the lack of error messages.
Reorganize schema files into schema dir. Those files were previously in gripql directory.
Also removes all instances of elasticSearch from Grip. This include the website too.
Proto spec now abstracts project_id into extraArgs