-
Notifications
You must be signed in to change notification settings - Fork 189
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: jsonformat package is not go gettable #18
Comments
Great idea. We have a maven jar for Java, and will have pypi support for Python, so it makes sense to also support go modules. We'll investigate making this coexist with bazel - as long as that plays nicely we should be able to get this out soon. |
I want to emphasize what @sam-virta stated. I am trying to use your code to convert FHIR JSON to proto. It is very frustrating as every road leads through Bazel. My use case is that I would like to be able to run an AWS Lambda in any language that will convert FHIR JSON data to proto. So I learned GO just for the purpose of the conversion as I wanted to take advantage of your code. WWYD? Please advise and thank you for reading. Am I better off using the advice [here](https://stackoverflow.com/questions/38406211/how-to-convert-from-json-to-protobuf to accomplish) to accomplish my goal of converting JSON FHIR to Proto? |
This is a high priority for us given the feedback from this issue |
Quick update on this: I've made some internal changes in v0.5.4 that update our import paths (to the familiar github.com prefixed go-gettable paths) and code location, the first step in making the library go-gettable and go mod compliant. The next step will be to generate and include the go proto code in the repo and have the go libraries depend directly on them (instead of relying on the bazel-based proto code generation). :) |
@suyashkumar running into the following: main.go package main
import (
_ "github.com/google/fhir/go/jsonformat"
)
func main() {
}
Should we wait for .6 for Bazel-free usage? Related Go ticket: golang/go#40780 |
@loafoe We won't be able to fully support go modules until we generate and include the go proto code in the repo (as mentioned above, this is the next step). However, the repo size limitation you run into is interesting, thank you for raising this! I'm not super familiar with why our repo size is so large (@nickgeorge may know more): looking at the fhir-0.5.4.zip: https://github.com/google/fhir/archive/v0.5.4.zip (it seems like its around 111MB compressed, and ~900MB uncompressed). This may be a good reason to consider separating this library out from the rest of the FHIR repo if we will not be able to get around this limit, which will also be a blocker for adopting go modules. We'll discuss some more internally on next steps and go from there. Thank you! |
hmm is the go module size just the size of the whole repo? If it's just the go code, I'd be pretty surprised if it crossed some threshold. As for the repo itself - my guess is most of that is the versions of the FHIR spec itself we have checked in, for use in proto generation. I have a plan to run the proto generation off of MPM location as published by FHIR, at which point I'd likely expunge the checked-in versions of the spec from the git history. That should result in a pretty significant drop in overall size. |
Thanks @nickgeorge! Makes sense, it seems like the checked in spec is indeed ~700MB. Because go modules (and go get) rely on cloning the root repository to fetch and build the dependency, that's probably why it's running into an issue. One thing to note: while go modules may have this issue, |
Is there any way to generate the proto files ourselves and include them in a Go project so we can use the jsonformat package? If there is, are there instructions on how to do that? |
Hi @wesdean! I wouldn't say this is recommended, but to answer your question: you can always use the protocol buffer compiler Keep in mind that if you use Bazel as a build system for your project, you should not have to worry about any of this (Bazel will handle building the jsonformat package and building the required proto dependencies, though there are some setup costs with Bazel). In terms of where we are with |
Hi folks, with the release of v0.7.0, we have You can check out the updated FHIR examples that now also use Go modules, and you can start using Let us know if you have any questions. |
Feel free to reopen if you encounter any issues |
I am getting this error. Any ideas on how to resolve? |
Thanks for mentioning this! Can you give us a little more context on your setup? In particular your Go version, and if you're setting any environment variables (e.g. GOPROXY)? The easiest way to debug for us would be to try to set up a minimal repository that can reproduce this error and start from there. A minimal working example with go modules can be seen here (clone into your GOPATH, and you should be able to Out of curiosity, does the minimal working example above work with your configuration (e.g. GOPROXY, etc?) |
Thanks @suyashkumar. I solved it. I was trying to import |
Great to hear! Let us know if you run into any other issues. Thanks! |
Sorry to be a bother, but I'm having some trouble when serializing a Bundle. I'm getting an error on the Birth Date field of a Patient record. When I set the Birth Date to nil, I get other serialization errors such as I get these errors when my service starts up. Could my issue have something to do with the bundle being from the Thanks |
FHIR proto timezones need to specified as an offset, i.e. "-08:00"
We only support the Google FHIR protos in with this library. Some protos may happen to work by nature of reflection similarities, but this marshaller is specifically for our our protos. |
@wesdean Please open another issue if you have further questions and we'll be happy to help you there |
Currently the imports in the jsonformat go package require consumers to also be using bazel. It would be nice if this implementation was directly consumable (as in, via go modules)
If you aren't planning on supporting this, please let me as we will try a go mod hack or begrudgingly maintain a fork 🙂
The text was updated successfully, but these errors were encountered: