Conversation
quinnwai
left a comment
quinnwai
left a comment
There was a problem hiding this comment.
Couple comments, what do I need to test?
| RUN git clone https://github.com/bmeg/iceberg.git && \ | ||
| cd iceberg && \ | ||
| git checkout feature/FHIR-resource-type | ||
| git checkout 7f6cfdb558d05370fc645b5ab894b98b38a01e1b |
There was a problem hiding this comment.
Why not just main or the most recent commit in iceberg? If there's a tendency to not do main then we should have a development branch or something in iceberg to pin it to
There was a problem hiding this comment.
because the server is setup to use a very specific version of iceberg. See gen3-helm/helm/grip/templates/post-something.yaml
I want to match what I know the server is using.
| project_str_dict = f'{{"auth_resource_path":"/programs/{program}/projects/{project}"}}' | ||
| print(f"Using project: {project_str_dict}") | ||
| result = subprocess.run( | ||
| ["jsonschemagraph", "gen-dir", "iceberg/schemas/graph", f"{temp_dir}", f"{temp_dir}/OUT", "--extraArgs", project_str_dict, "--gzip_files"], |
There was a problem hiding this comment.
Wanted to build jsonschemagraph from main branch but when building jsonschemagraph from source, running into...
graphql/griptographql.go:29:8: v.Gid undefined (type *gripql.Vertex has no field or method Gid)
feature/update-grip-structs branch at least compiled and had the --extraArgs flag, how to proceed?
There was a problem hiding this comment.
Yeah that's because grip was using a shadow branch. See:
bmeg/jsonschemagraph#16 try with go install github.com/bmeg/jsonschemagraph@b88905d8d65d858be9b52e04632dde309f033a3e
which is referencing the version from bmeg/jsonschemagraph@b88905d
But for this PR I believe it's using a executable from this release https://github.com/bmeg/jsonschemagraph/releases/tag/v0.0.3 if you check the Dockerfile
There was a problem hiding this comment.
okay so to confirm: all dockerfile dependencies point to merged branches for iceberg and jsonschema?
There was a problem hiding this comment.
forgot about this repo. Had to make a new PR now that you mentioned it. bmeg/jsonschemagraph#16. Since it's working, if you want to review it you can, but not going to make any changes to it that impact this release since it's working.
There was a problem hiding this comment.
eems like there's at least one other dependency github.com/bmeg/grip v0.0.0-20250421161012-b9b392fc8721 that is from an open PR.
Since it's statically compiled in Go, I'm less worried about updates to the PR causing breaking changes but I want us to be more stringent in future releases: all dependencies should point to merged code if it's a feature in the release.
I'm good to proceed if we start enforcing this rule in future releases. How do I test these changes?
There was a problem hiding this comment.
grip is using this jsonschemagraph directly as a package. if jsonschemagraph doesn't work you will know it in downstream code repositories, and ultimately in the g3t end to end test
Kyle had approved this PR in grip, that requires this dependency. You test these changes using the g3t end to end test like what is specified int he release notes
| server_errors.append(res[0]["message"]) | ||
| else: | ||
| print(f"jsonschemagraph failed with exit code: {result.returncode}") | ||
| print("Stdout:") |
There was a problem hiding this comment.
just confirming, the stdout gets logged to the FHIR server pod logs?
There was a problem hiding this comment.
Any error logs will come as a https response message. Any more in depth logs can be checked by checking the fhir server logs in k8s.
quinnwai
left a comment
quinnwai
left a comment
There was a problem hiding this comment.
looks good just dunno how to test it. What are the steps to do so?
|
calypr.ohsu.edu should be updated already iirc |
|
Oh yeah you're right that is' on |
2 participants
No description provided.