org.hl7.fhir.core icon indicating copy to clipboard operation
org.hl7.fhir.core copied to clipboard

Enable bulk -snapshot and -convert on multiple/wildcard -source

Open david-simons opened this issue 2 years ago • 7 comments

  • builds successfully (org.hl7.fhir.validation.cli-5.6.70-SNAPSHOT.jar currently)
  • the two fixes on http to https are because of 501 HTTPS Required. Use https://repo.maven.apache.org/maven2/. More information at https://links.sonatype.com/central/501-https-required when building via mvn -DfhirTxcacheRebuild -DskipTests clean install
  • updated help.txt
  • Simplified-up terminal output:
PS vitalSign-v1> java -jar validator_cli.jar *.structuredefinition.xml -version 4.0.1 -jurisdiction uv -snapshot -output snapshot.json
FHIR Validation tool Version 5.6.70-SNAPSHOT (Git# 17d265b9d81d). Built 2022-10-07T22:31:09.77+02:00 (25 mins old)
  Java:   17 from C:\Program Files\jdk-17 on amd64 (64bit). 2018MB available

  Paths:  Current = vitalSign-v1, Package Cache = .fhir\packages     
  Params: BloodPressureVitalSign.structuredefinition.xml BMIVitalSign.structuredefinition.xml BodyTemperatureVitalSign.structuredefinition.xml HeadCircumferenceVitalSign.structuredefinition.xml HeartRateVitalSign.structuredefinition.xml HeightVitalSign.structuredefinition.xml OxygenSaturationVitalSign.structuredefinition.xml RespiratoryRateVitalSign.structuredefinition.xml VitalSign.structuredefinition.xml WeightVitalSign.structuredefinition.xml -version 4.0.1 -jurisdiction uv -snapshot -output snapshot.json
  Jurisdiction: No Jurisdiction
Loading
  Load FHIR v4.0 from hl7.fhir.r4.core#4.0.1 - 4575 resources (00:06.628)
  Load hl7.terminology#4.0.0 - 4164 resources (00:02.135)
  Load R5 Extensions - 85 resources (00:11.339)
  Terminology server http://tx.fhir.org - Version 2.0.14 (00:01.275)
  Get set...  go (00:00.050)
 ...generated snapshot [0] successfully
 ...generated snapshot [1] successfully
 ...generated snapshot [2] successfully
 ...generated snapshot [3] successfully
 ...generated snapshot [4] successfully
 ...generated snapshot [5] successfully
 ...generated snapshot [6] successfully
 ...generated snapshot [7] successfully
 ...generated snapshot [8] successfully
 ...generated snapshot [9] successfully
Done. Times: Loading: 00:21.812. Max Memory = 1Gb
PS vitalSign-v1> 

Resulting files: image

david-simons avatar Oct 07 '22 21:10 david-simons

Codecov Report

Base: 9.60% // Head: 9.59% // Decreases project coverage by -0.00% :warning:

Coverage data is based on head (b21f09f) compared to base (b17735a). Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #945      +/-   ##
============================================
- Coverage      9.60%    9.59%   -0.01%     
  Complexity    13189    13189              
============================================
  Files          2432     2432              
  Lines        972320   972347      +27     
  Branches     289097   289105       +8     
============================================
+ Hits          93344    93345       +1     
- Misses       842924   842951      +27     
+ Partials      36052    36051       -1     
Impacted Files Coverage Δ
.../org/hl7/fhir/validation/cli/model/CliContext.java 0.00% <0.00%> (ø)
...hir/validation/cli/services/ValidationService.java 0.00% <0.00%> (ø)
...java/org/hl7/fhir/validation/cli/utils/Params.java 0.00% <0.00%> (ø)
.../org/hl7/fhir/r5/utils/structuremap/Variables.java 82.14% <0.00%> (-0.62%) :arrow_down:
...org/hl7/fhir/r4b/utils/structuremap/Variables.java 0.00% <0.00%> (ø)
.../fhir/utilities/tests/CacheVerificationLogger.java 20.83% <0.00%> (+8.33%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Oct 07 '22 21:10 codecov[bot]

@grahamegrieve @dotasek : kindly requesting this to become part of the HL7 Validator release - it brings our nightly build down from many hours down to a few minutes - by being able to use wildcards on -source for the -convert and for the -snapshot operations.

david-simons avatar Oct 10 '22 12:10 david-simons

It seems to me that the output files generated here will have names decoupled from their sources.

In your example, BloodPressureVitalSign.structuredefinition.xml could be any one of [x].snapshot.json. which may work when loading the whole directory, but could make debugging a pain.

What I would suggest is adding another option, perhaps -outputSuffix that gets used in the place of -output in the case where there is more than one source.

So, for example using -outputSuffix .snapshot.xml, you would get BloodPressureVitalSign.structuredefinition.xml.snapshot.xml.

Does this seem reasonable?

Indeed that is a concern - yet I am not a developer - so wanted to make minimal, and only local changes to the code. Adding an additional parameter is a lot to ask. ;) So I'd like to stick with the existing -source and -output parameter, for this.

We are considering to use a Map (in our wrapper script) of {source-filename, Resource.id / StructureDefinition.url} to map the output files back to the source files.

Perhaps the ...generated snapshot [0] successfully can be expanded to something like ...generated snapshot [0] successfully from BloodPressureVitalSign.structuredefinition.xml to [0].snapshot.json

What might also work is to use the existing -output String: BloodPressureVitalSign.structuredefinition.xml.snapshot.json when assuming the -source files are uniquely named (a fair assumption).

Let us give this a try...

UPDATE: Done making the requested enhancements, @dotasek :)

  Get set...  go (00:00.067)
 ...generated snapshot [0] successfully (BloodPressureVitalSign.structuredefinition.xml to BloodPressureVitalSign.structuredefinition.xml.snapshot.json)
 ...generated snapshot [1] successfully (BMIVitalSign.structuredefinition.xml to BMIVitalSign.structuredefinition.xml.snapshot.json)
 ...generated snapshot [2] successfully (BodyTemperatureVitalSign.structuredefinition.xml to BodyTemperatureVitalSign.structuredefinition.xml.snapshot.json)
 ...generated snapshot [3] successfully (HeadCircumferenceVitalSign.structuredefinition.xml to HeadCircumferenceVitalSign.structuredefinition.xml.snapshot.json)
 ...generated snapshot [4] successfully (HeartRateVitalSign.structuredefinition.xml to HeartRateVitalSign.structuredefinition.xml.snapshot.json)
 ...generated snapshot [5] successfully (HeightVitalSign.structuredefinition.xml to HeightVitalSign.structuredefinition.xml.snapshot.json)
 ...generated snapshot [6] successfully (OxygenSaturationVitalSign.structuredefinition.xml to OxygenSaturationVitalSign.structuredefinition.xml.snapshot.json)
 ...generated snapshot [7] successfully (RespiratoryRateVitalSign.structuredefinition.xml to RespiratoryRateVitalSign.structuredefinition.xml.snapshot.json)
 ...generated snapshot [8] successfully (VitalSign.structuredefinition.xml to VitalSign.structuredefinition.xml.snapshot.json)
 ...generated snapshot [9] successfully (WeightVitalSign.structuredefinition.xml to WeightVitalSign.structuredefinition.xml.snapshot.json)
Done. Times: Loading: 00:21.019. Max Memory = 1Gb

image

  Get set...  go (00:00.060)                                                                                                  apefinition.xml.snapshot.json)
 ...convert [0] (BloodPressureVitalSign.structuredefinition.xml to BloodPressureVitalSign.structuredefinition.xml.convert.json)                                                                                                                             urshot.json)
 ...convert [1] (BMIVitalSign.structuredefinition.xml to BMIVitalSign.structuredefinition.xml.convert.json)
 ...convert [2] (BodyTemperatureVitalSign.structuredefinition.xml to BodyTemperatureVitalSign.structuredefinition.xml.convert.edjson)
 ...convert [3] (HeadCircumferenceVitalSign.structuredefinition.xml to HeadCircumferenceVitalSign.structuredefinition.xml.conv  ert.json)                                                                                                                     ap
 ...convert [4] (HeartRateVitalSign.structuredefinition.xml to HeartRateVitalSign.structuredefinition.xml.convert.json)
 ...convert [5] (HeightVitalSign.structuredefinition.xml to HeightVitalSign.structuredefinition.xml.convert.json)
 ...convert [6] (OxygenSaturationVitalSign.structuredefinition.xml to OxygenSaturationVitalSign.structuredefinition.xml.convert.json)
 ...convert [7] (RespiratoryRateVitalSign.structuredefinition.xml to RespiratoryRateVitalSign.structuredefinition.xml.convert.json)
 ...convert [8] (VitalSign.structuredefinition.xml to VitalSign.structuredefinition.xml.convert.json)
 ...convert [9] (WeightVitalSign.structuredefinition.xml to WeightVitalSign.structuredefinition.xml.convert.json)
Done. Times: Loading: 00:20.344. Max Memory = 1Gb

image

david-simons avatar Oct 12 '22 07:10 david-simons

@david-simons one more consideration, which I'll have to check myself: what happens with a mixture of input files?

BloodPressureVitalSign.structuredefinition.xml
BMIVitalSign.structuredefinition.json
...

I will have to check the logic, but I believe the output will match formats, meaning the suffix will have to change for each file:

BloodPressureVitalSign.structuredefinition.output.xml
BMIVitalSign.structuredefinition.output.json
...

The argument would simply be -outputSuffix output in this case.

dotasek avatar Oct 13 '22 22:10 dotasek

@david-simons one more consideration, which I'll have to check myself: what happens with a mixture of input files?

I checked, and the conversion is actually determined by the single -output parameter, not by the mixed input! So it is predicteable.

Example: a combination of xml+json input will be converted to .json, when -convert -output convert.json (determined by the type out the -output parameter, nicely) (I checked the contents, also a roundtrip back to a snapshot.xml works)

The help.txt explains the "context" differences, in the -source -output combinations. Note that the parameter already is generically output, not outputFilename :)

vitalSign-v1> java -jar C:\Users\nlv09099\Source\Repos\org.hl7.fhir.core-fork\org.hl7.fhir.validation.cli\target\org.hl7.fhir.validation.cli-5.6.71-SNAPSHOT.jar .\BloodPressureVitalSign.structuredefinition.xml .\BloodPressureVitalSign.structuredefinition.json -version 4.0.1 -jurisdiction uv -convert -output convert.json       
FHIR Validation tool Version 5.6.71-SNAPSHOT (Git# dd65b1a40fa6). Built 2022-10-12T10:26:39.911+02:00 (46 hours old)
  Java:   17 from C:\Program Files\jdk-17 on amd64 (64bit). 2018MB available

Loading
<cut>
  Get set...  go (00:00.048)
 ...convert [0] (.\BloodPressureVitalSign.structuredefinition.xml to .\BloodPressureVitalSign.structuredefinition.xml.convert.json)
 ...convert [1] (.\BloodPressureVitalSign.structuredefinition.json to .\BloodPressureVitalSign.structuredefinition.json.convert.json)
Done. Times: Loading: 00:20.986. Max Memory = 1Gb

image

david-simons avatar Oct 14 '22 07:10 david-simons

Thanks - I merged in your additional commit ( Tests and add new parameter), @dotasek, and @cdejonge and I will give these a try in our pipeline asap

update: a first thought is that we should allow for a single source file also with --outputSuffix, since in our case sometimes a wildcard -source *.xml just translates to a single source file, yet we do want to use the --outputSuffix filenaming for consistency in the pipeline.

david-simons avatar Oct 16 '22 19:10 david-simons

Have a look at my additional commits on this branch @dotasek - adding logic for -output XOR -outputSuffix - tested also. This will allow us to use the -outputSuffix also with a single -source file; next to of course using -output with a single -source file. The choice is determined by using either -output XOR -outputSuffix.

help.txt also updated

david-simons avatar Oct 17 '22 11:10 david-simons