Stream: committers
Topic: retooling the core build
Grahame Grieve (Jan 06 2021 at 22:28):
ok. I'm getting somewhere. Frustratingly slowly, but somewhere.
Grahame Grieve (Jan 06 2021 at 22:28):
FYI, here's a copy of the retooled spreadsheets
Grahame Grieve (Jan 06 2021 at 22:29):
Grahame Grieve (Jan 06 2021 at 22:31):
they're a little different, since there's some changes under the hood.
Jose Costa Teixeira (Jan 07 2021 at 00:16):
are these the ones that can be converted back and forth to StructureDefs?
Jose Costa Teixeira (Jan 07 2021 at 00:18):
I presume there would also be a spreadsheet for defining the valuesets...?
Possibly the Profiles tab will be added as well, I presume.
And final question, when can we play with it? I want to create a DeviceDispense resource and I'll gladly give this a try if that is somehow useful.
Grahame Grieve (Jan 07 2021 at 05:45):
no. valuesets and code systems are done natively as resources once I introduce this.
Grahame Grieve (Jan 07 2021 at 05:45):
so are profiles
Vassil Peytchev (Jan 07 2021 at 18:16):
Natively, as in editing JSON files for each resource?
Lloyd McKenzie (Jan 07 2021 at 18:58):
Or XML, I hope...
Grahame Grieve (Jan 15 2021 at 11:10):
ok. this is now committed and live, so the editing process has changed.
Grahame Grieve (Jan 15 2021 at 11:11):
up until now, the master definition of the resources has been in a bunch of xml files that are excel spreadsheets. All of the resources part of the build was generated from these spreadsheets
Grahame Grieve (Jan 15 2021 at 11:13):
well, no more. From today, the master definitions will be stored as a series of resources:
- a structure definition for the resource itself
- a bundle of search parameters for the search parameters
- 3 lists, for examples, operations, profiles
- a series of operation definitions, value sets and code systems that support these
Grahame Grieve (Jan 15 2021 at 11:14):
These are committed to version control instead of the old xml spreadsheets, which have been removed (for the resources; the data type spreadsheets have not been changed)
Grahame Grieve (Jan 15 2021 at 11:16):
However, you can still edit the resources using excel. The way this works is that when you run the build, it reads the master definition resources, and creates an excel spreadsheet on the fly (unless it's already open in excel, in which case the file is locked)
Grahame Grieve (Jan 15 2021 at 11:16):
then, when you run a build, the build will check the date of the spreadsheet; if it's been changed since it was written, it will read it, merge the information in it back into the master resources, and then use the updated resources.
Grahame Grieve (Jan 15 2021 at 11:17):
you don't need to close excel for that to happen. (though you do need to close it if you do a pull, and need the excel spreadsheet updated from new definitions, and I have no way to check on this automatically)
Grahame Grieve (Jan 15 2021 at 11:20):
the format of the spreadsheets has changed a little. The biggest change is that you no longer can define code systems / value sets for required bindings in the spreadsheet; you do it as native code system and value set resources. The same applies for profiles, extensions etc - these are defined in the native resource format. There's some more subtle changes in the element definitions - the spreadsheet is much more aligned with the formal definitions of the resources now, so you might not really notice the other changes
Grahame Grieve (Jan 15 2021 at 11:20):
you can edit the master resources directly, if you prefer.
Grahame Grieve (Jan 15 2021 at 11:20):
the spreadsheets are never committed to github
Grahame Grieve (Jan 15 2021 at 19:08):
@Josh Mandel a question about R4B, since I'm now working on that - should that be worked on in a branch of the main repo, or should it get it's own repo?
My preferred answer is: it should have it's own repo, and it's own ci-build.
Josh Mandel (Jan 15 2021 at 19:11):
Certainly your call. The nice thing about using a branch is that the existing infrastructure would just keep working; what are the advantages to a separate repository?
Grahame Grieve (Jan 15 2021 at 19:18):
It's certainly easier to keep the same infrastructure for the infrastructure maintainers, sure. But I don't think it's easier for the editors.
Grahame Grieve (Jan 15 2021 at 19:20):
for editors, the flow should be the same: commit -> PR to main -> checked PR ok -> commit to master.
Its not the same group of editors either - a smaller subset.
Josh Mandel (Jan 15 2021 at 20:07):
Sure, but substitute r4b for main in the flow above, and it's identical for committers. Re: smaller subset of editors, we've never enforced this kind of restriction before but if desired we could manage and enforce this with branch level restrictions like:
Josh Mandel (Jan 15 2021 at 20:08):
(I'm not set against making a separate repo, but I want to make sure we've thought it through.)
Grahame Grieve (Jan 15 2021 at 22:07):
well, the principle issue, so far as I can see, is that as far as I know, the ci-build doesn't post r4b to build.fhir.org. At least it doesn't at the moment. I guess if that happened, it wouldn't make much difference which repo it is
Grahame Grieve (Jan 18 2021 at 21:07):
@Josh Mandel ping on this - need resolution in the next few days
Grahame Grieve (Jan 18 2021 at 21:08):
Note about an R4B limitation: the example value set valueset-nhin-purposeofuse.xml includes a concept map, and the R5 build can't read that as a value set, since concept map has been changed. so that value set won't be listed with other value sets, though it will still appear as an example against the value set resource
Josh Mandel (Jan 18 2021 at 21:14):
Interesting -- I'm out for the holiday weekend but back tomorrow. I'd expect r4b to appear in the branch build at /branches, unless there's something causing it to break.
Grahame Grieve (Jan 18 2021 at 21:15):
I didn't know about /branches and couldn't find any reference to it. Can we update the PR build notification to include the link?
Josh Mandel (Jan 19 2021 at 04:03):
http://build.fhir.org/branches/gg-big-rework is building, but I'm not sure which branch contains "r4b" (not seeing an active branch with that name); can you include a link here?
Grahame Grieve (Jan 19 2021 at 04:17):
I haven't committed it yet. It will be r4b once I do
Josh Mandel (Jan 19 2021 at 17:21):
Oh, OK -- well, I'm hoping it will "just work". When you wrote earlier:
as far as I know, the ci-build doesn't post r4b to build.fhir.org.
... obviously it doesn't post r4b to the build site until you push r4b to github ;-))
Lloyd McKenzie (Jan 19 2021 at 17:23):
Branch builds go poof after 30 days of no new commits, right? Will that be an issue here?
Josh Mandel (Jan 19 2021 at 17:27):
We can set rules as needed.
Josh Mandel (Jan 19 2021 at 17:28):
I'm not expecting r4b is going to go inactive for >1mo anytime very soon though; first thing is to get it started.
Grahame Grieve (Jan 20 2021 at 10:09):
ok, tomorrow I'm committing an update to the main build (master) which does 2 things:
- fixes up a bunch of outstanding issues with the retooled R5 candidate (the master branch)
- also is able to run the R4B branch
I'll also be committing the R4B branch tomorrow - that's the R4 equivalent using the new formats. e.g. you stilll get spreadsheets if you build locally, just as it works for the R5 branch.
However, you don't build the R4B branch using the java code in R4B itself. Instead, you use the R5 java code to run it. Once I commit, you'll be able to run R4B by hacking the R5 build (which is how I'm doing it now). However, @Mark Iantorno is working on moving the java code out of the FHIR repo all together, and instead we'll use a FHIR publisher jar (like the IG publisher, based on similar infrastructure) to run on either R5 or R4B. Mark will provide updates as he gets that infrastructure in place (though for users + ci-build, it'll be just the same as now: run publish.bat/publish.sh)
One consequence of all this is that we won't have a ci-build for R4B until Mark gets his changes in place
Grahame Grieve (Jan 20 2021 at 10:13):
note that the R4B build has many broken links, and many validation failures. I've set it to run to completion irrespective of this, until I can address all the issues after the WGM.
Grahame Grieve (Jan 21 2021 at 02:49):
ok R4B branch now exists. And will be built by the code in the master branch
Grahame Grieve (Jan 21 2021 at 03:41):
ok. @Mark Iantorno here's where I am now:
- I have fixed a few things in the test cases - these will need releasing before fhir.core is next released
- fhir.core needs a release to support the publisher
- the java code in the gg-r4b-work-2 branch of fhir.main is the code that will publish both R4B and R5. The PR for that fails because fhir.core needs a release - I think. It works for me locally.
- the branch R4B in fhir.main has the source for R4B that will build with the java code in gg-r4b-work-2 branch, and is the base for R4B. It can't pass ci-build until you commit changes for it to build with the jar you are working on
- All this works for me and is committed, but is waiting for you to resolve build/release/new jar in order to complete
Mark Iantorno (Jan 21 2021 at 03:47):
I will take care of all this tomorrow. Thanks for letting me know.
Grahame Grieve (Jan 21 2021 at 03:48):
everyone, we have a name clash that has suddenly become confusing
Grahame Grieve (Jan 21 2021 at 03:51):
we have the 'core build' or 'fhir core' which is either the spec at http://hl7.org/fhir or the source repo, or the process that builds it, and we also have 'fhir core' which is the java code that sits underneath the validator, the ig publisher, hapi and smile-cdr, and also the 'fhir core' java code.
As you can see, that's suddenly confusing now that we're re-organizing things.
Grahame Grieve (Jan 21 2021 at 03:51):
I'm not sure what the right solution to that is
Lloyd McKenzie (Jan 21 2021 at 03:58):
Well, the bit that publishes core should probably be the "fhir core publisher".
Lloyd McKenzie (Jan 21 2021 at 04:00):
I think context will generally make clear whether someone's talking about the spec or the code when referring to "fhir core". When you refer to the 'fhir core' java code - do you mean the 'model' Java files and other stuff generated from the core spec?
Lloyd McKenzie (Jan 21 2021 at 04:00):
Could refer to those as the "fhir core java release" files - given that they vary from one release to another.
Rob Hausam (Feb 18 2021 at 04:28):
I haven't seen anyone posting about this in regard to the way spreadsheet editing works now, so maybe I'm doing something wrong with it (although I don't think so?). From @Grahame Grieve :
However, you can still edit the resources using excel. The way this works is that when you run the build, it reads the master definition resources, and creates an excel spreadsheet on the fly (unless it's already open in excel, in which case the file is locked)
then, when you run a build, the build will check the date of the spreadsheet; if it's been changed since it was written, it will read it, merge the information in it back into the master resources, and then use the updated resources.
you don't need to close excel for that to happen. (though you do need to close it if you do a pull, and need the excel spreadsheet updated from new definitions, and I have no way to check on this automatically)
I'm working with @François Macary on updates to ObservationDefinition, and what I'm seeing with the build doesn't seem to be matching this process. Francois has updated the new version of the spreadsheet for ObservationDefinition with the desired changes - several new and re-organized elements and updated text including definitions and comments.I have an up-to-date and clean build before trying to make the changes. I then replace the previously generated observationdefinition-spreadsheet.xlsx file with the updated version from Francois. This change appears to be detected correctly by the build, as it triggers a partial build of observationdefinition as expected. But what's happening next isn't what I expected. The build successfully completes, but the result is that rather than picking up and merging the updates from the spreadsheet into the StructureDefinition, the StructureDefinition is left unchanged and instead the spreadsheet file is being regenerated with the contents that it previously had (matching the StructureDefinition) and the updates to the resource do not get made. I have been closing the spreadsheet before running the build (as I still have had that habit from before). I wouldn't think that could cause a problem, but who knows. I'm not seeing anything obvious that I can do differently that would make this work - any advice is appreciated.
Jean Duteau (Feb 18 2021 at 04:56):
the build uses a timestamp to determine if the spreadsheet is newer than the structure definition or not. so if the file that you get from Francois doesn't have a proper timestamp, then the build doesn't treat it as new.
Grahame Grieve (Feb 18 2021 at 05:08):
yes I had not anticipated that work flow, and I think Jean is right
Rob Hausam (Feb 18 2021 at 21:15):
How could Jean be right? :) Actually, I certainly agree with Jean that the build uses a timestamp to determine if the spreadsheet is newer than the structure definition, which is what I thought I described in my post. That part seemed to work, as when the spreadsheet was updated that triggered the partial build, presumably because it did recognize the timestamp. I don't understand the last part of the comment about the file from Francois "doesn't have a proper timestamp", as my assumption was that the timestamp that the build saves for comparison (in observationdefinition-spreadsheet.datestamp) was being compared with the time that the file was modified in the filesystem - which is independent of any of the file contents. So if that's not the timestamp that is being compared then I don't know which one it is that needs to be "proper" (unless you are referring to meta.lastUpdated, which I don't think would make sense). So, as I said, the partial build was actually triggered by the spreadsheet update, but the behavior still was different than I expected. And to me this does seem to me to be a typical workflow that we should expect to handle. So what else am I still missing? @Grahame Grieve @Jean Duteau
Gino Canessa (Feb 18 2021 at 21:19):
File timestamps between computers can be.... tricky.
Rob Hausam (Feb 18 2021 at 21:21):
Yes, I agree with that. But I think this is all done locally (nothing checked into GH), so the timestamps should be consistently comparable?
Gino Canessa (Feb 18 2021 at 21:22):
I think the problem is in the "replace the file with the one from Francois" step. File timestamps can be mangled / changed / removed / etc. How are you getting that updated version?
Jose Costa Teixeira (Feb 18 2021 at 21:25):
Not sure if it helps, but when I was rinning a build, I saved my xlsx right after saving the StructureDef, and of course that gave me an issue.,
Jose Costa Teixeira (Feb 18 2021 at 21:25):
Also - are timestamps universal across timezones? I presume François is in another timezone - so if François saves a file now, and you change it in 3 hours, François's file is perhaps still saved "later" than yours
Josh Mandel (Feb 18 2021 at 21:25):
You can try running touch to modify file creation dates (just from a debugging perspective; once we understand what's going on, we can presumably refine the workflow)
Gino Canessa (Feb 18 2021 at 21:25):
One example (even in a single Windows system) is that the NTFS filesystem stores everything in UTC and the FAT filesystem stores everything in system local time.
Edit: Note: this gets more complicated when changing operating systems.
Josh Mandel (Feb 18 2021 at 21:26):
I forget if you're on a Mac, Rob. I just read https://apple.stackexchange.com/a/187673 and am always impressed with how much I don't know :-)
Rob Hausam (Feb 18 2021 at 21:27):
Yes, I see what you mean. If Francois updated the spreadsheet on his machine and then uploads it (to Skype in this case), and then I download it and save it on my machine it would be keeping the timestamp from Francois' machine unless I actually make an additional update to the file locally? That could be the reason, and I can try to test that. And yes, I'm on a Mac (and I expect Francois is on Windows).
Jose Costa Teixeira (Feb 18 2021 at 21:29):
I'd just open, add whitespace and save again
Rob Hausam (Feb 18 2021 at 21:29):
That's exactly what I was getting ready to try right now.
Gino Canessa (Feb 18 2021 at 21:29):
On the mac, this StackOverflow answer looks helpful - essentially an automation to update the file time.
Rik Smithies (Feb 18 2021 at 21:42):
Did you notice the {resourcename}-spreadsheet.timestamp files? I assumed that was the key to this.
Rob Hausam (Feb 18 2021 at 21:44):
Yes, to all of it. That was in fact the issue. Making a local update to the file solves that problem. It's just a bit of a pain and not terribly obvious that it needs to be done, though. Appreciate everyone's suggestions.
Last updated: Apr 12 2022 at 19:14 UTC
