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

go: profiling + validation support in go #22

Open
tmc opened this issue Sep 10, 2020 · 11 comments
Open

go: profiling + validation support in go #22

tmc opened this issue Sep 10, 2020 · 11 comments

Comments

@tmc
Copy link

tmc commented Sep 10, 2020

We're maintaining some custom resource validation code that we'd love to deprecate in favor of profiling support in this codebase.

Can you give an estimate on when we might see profiling+validation support for go in this repo?

@nikklassen
Copy link
Member

What kind of profiling/validation are you looking for as part of these libraries? Some is already baked in, and we'd like to separate it out explicitly. Full validation of a StructureDefinition is not in the scope of this library right now.

@nickgeorge
Copy link
Collaborator

Hi Travis,
To expand a bit on what Nik said, there's a couple of things to keep in mind (sorry if you know all of this!)

Profiles are generated using a standalone process - these are essentially meta descriptions that describe your data, so they're typically not modified in-process. The code itself is written in Java, but that doesn't matter a whole lot, you can use the generated profile protos in any language you like.

Similarly, the validation libraries themselves don't particularly care if something is a "profile" or not - they use the proto annotations in the same ways to validate.

In current state, Go kind of lumps validation and parsing together in a way that makes it hard to use separately - it's definitely on the roadmap to separate those two, but it's a bit hard to say exactly when that would happen - I'd guess next quarter but maybe Nik can give a better estimate.

So the final piece to all this puzzle is the profiler itself, which I'm guessing is the core of your question - this is the ability to say "Take data in core resources, and convert it to this profile." Unfortunately, that's gonna take porting a decent amount of code from C++ and isn't currently a huge priority for the Go folks so I'm not sure I can promise it in the immediate future.

@tmc
Copy link
Author

tmc commented Sep 10, 2020

Thanks for the responses. To clarify, we're generating protos using the java code in the repository but and the things I'm after in terms of go support are the validation of fields (e.g. REQUIRED_BY_FHIR) as well as code for conversion between a profiled and non-profiled representation of resources.

It sounds like some of the validation code is in the pipes but we may be on the line for maintaining code to perform conversions to/from profiled representations. Is that accurate?

@nickgeorge
Copy link
Collaborator

Yeah that sounds right to me. Maybe you could do some kind of swig style wrapper around https://github.com/google/fhir/blob/master/cc/google/fhir/r4/profiles.h ?

@nikklassen
Copy link
Member

The Go code does support the validation fields like "required by FHIR", but only if it's the Go code parsing from JSON to proto. Like Nick said, validation of a Go proto struct that has already been parsed is probably coming in the next quarter. The profiled to unprofiled conversion hasn't been a feature we considered for Go yet, so if you can use the C++ code that would be your best bet.

@tmc
Copy link
Author

tmc commented Sep 10, 2020

AFAICT that code doesn't work today with new Go types generated from profiles (e.g. won't be in the list of possible oneofs for ContainedResource). Is that true?

@nickgeorge
Copy link
Collaborator

ContainedResource behavior is governed by the PackageInfo#contained_resource oneof: https://github.com/google/fhir/blob/master/proto/profile_config.proto#L26
If you set local_contained_resource to true, it will generate a new ContainedResource to use throughout the profile. Make sure to add a Bundle profile to your profile set so you have a Bundle that has Entry with the right ContainedResource.

Exciting to hear about external usages of profiles!

@nickgeorge
Copy link
Collaborator

Although thinking a bit more, you might have limited library support after you convert... unlike the C++ library, which is reflective + generic, some APIs in the Go library expect core profiles. But if you wrap the "profile + validate" C++ APIs, you should be able to get solid validation reports back from your protos, and then manipulate them in Go.

@tmc
Copy link
Author

tmc commented Sep 10, 2020

ContainedResource behavior is governed by the PackageInfo#contained_resource oneof: https://github.com/google/fhir/blob/master/proto/profile_config.proto#L26
If you set local_contained_resource to true, it will generate a new ContainedResource to use throughout the profile. Make sure to add a Bundle profile to your profile set so you have a Bundle that has Entry with the right ContainedResource.

Exciting to hear about external usages of profiles!

Oh we overlooked that! Thanks for pointing that out. I'm guessing currently the jsonformat apis aren't going to be compatible with an alternate ContainedResource type (as I think you're hinting at).

@nickgeorge
Copy link
Collaborator

yeah, that's right, that's why I'm recommending doing the parsing and validating in C++ for profiles.

@nikklassen
Copy link
Member

As of 0.6.3 we have launched the fhirvalidate package that can enforce checks like required fields in Go without going through the parser.

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

No branches or pull requests

3 participants