new import tool #3182

v-mserdar · 2023-03-17T17:48:10Z

Description

This adds an import tool for use with either fhir oss or paas. It can also monitor the status of an ongoing import.
Read the readme.md file for a complete description of what this can do.

Related issues

Addresses [issue #].

Testing

It was tested while importing more than 1 TB of data to a fhir endpoint.

FHIR Team Checklist

Update the title of the PR to be succinct and less than 65 characters
Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
CI is green before merge
Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

SergeyGaluzo · 2023-03-20T18:52:14Z

tools/ImporterV2/ImporterV2.cs

+
+namespace Microsoft.Health.Fhir.ImporterV2
+{
+    internal static class ImporterV2


ImporterV2

Please reflect usage of this tool in its name.
It uses $import, registers it, and monitors it progress.
I can see that it can fail in monitoring. Tool should provide option to monitor without registering.

SergeyGaluzo · 2023-03-20T18:52:44Z

tools/ImporterV2/App.config

+		<add key="grant_type" value="Client_Credentials" />
+		<add key="client_id" value="{add your client id}" />
+		<add key="client_secret" value="{add your client secret}" />
+		<add key="resource" value="{add your FHIR endpoint}" />


Why it is not jus a connection string?

It uses the connection string for getting the blob container and lines 25-28 in the app.config for getting the auth token needed for $import status. Please let me know if I'm misunderstanding your question.

Will it work the same way for OSS FHIR?

SergeyGaluzo

🕐

…en getting import status

SergeyGaluzo · 2023-03-31T20:21:50Z

tools/ImporterV2/readme.md

+1. It can monitor a long running $import job by polling the status. Simply provide the MonitorImportStatusEndpoint and the token fields in the config file. 
+   Note that when this field is supplied then no other operation can be performed. Meaning you can't concurrently POST an $import. 
+   Consequently when this setting is empty then the tool looks to POST an $import job.
+2. Run an $import job by consuming ndjson blob files from the configured container and posting them to an OSS endpoint.


Run

Register

SergeyGaluzo · 2023-03-31T20:22:04Z

tools/ImporterV2/readme.md

+   Note that when this field is supplied then no other operation can be performed. Meaning you can't concurrently POST an $import. 
+   Consequently when this setting is empty then the tool looks to POST an $import job.
+2. Run an $import job by consuming ndjson blob files from the configured container and posting them to an OSS endpoint.
+3. Run an $import job by consuming ndjson blob files from the configured container and posting them to a Paas endpoint.


Run

Register

SergeyGaluzo · 2023-03-31T20:22:51Z

tools/ImporterV2/readme.md

+When running FHIR OSS be sure to have these configuration settings in the portal:  
+FhirServer__Operations__Import__Enabled = True  
+FhirServer__Operations__Import__InitialImportMode = True  
+FhirServer__Operations__Import__StorageAccountConnection = your_storageaccount_access_key_connection_string  


FhirServer__Operations__Import__StorageAccountConnection = your_storageaccount_access_key_connection_string

Remove

SergeyGaluzo · 2023-03-31T20:24:36Z

tools/ImporterV2/readme.md

+FhirServer__Operations__IntegrationDataStore__StorageAccountConnection = your_storageaccount_access_key_connection_string  
+
+**You may need to remove this setting to get import to work**
+FhirServer__Operations__IntegrationDataStore__StorageAccountUri


Can you test with leaving this setting and see if this still work?

This needs to be removed, so I'll update the comment stating so.

SergeyGaluzo · 2023-03-31T20:26:51Z

tools/ImporterV2/App.config

+    the type of files it'll search for in the container 
+    do not add a file extension as ndjson is assumed and added to the ResourceType automatically
+    -->
+    <add key="ResourceType" value="Observation" />


Please specify how to handle case when I don't want to have any filters.

I updated the comment stating that leaving this empty will search all.

SergeyGaluzo · 2023-03-31T20:30:32Z

tools/ImporterV2/App.config

+    of course this can be a much larger value than 20 so adjust as needed
+    -->
+    <add key="NumberOfBlobsForImport" value="20" />
+    <add key="FhirEndpoint" value="{add your FHIR endpoint}" />


{add your FHIR endpoint}

Why is this in 2 places?

I have removed this one

SergeyGaluzo

SergeyGaluzo

brendankowitz · 2023-04-11T17:52:44Z

tools/RegisterAndMonitorImport/RegisterAndMonitorImport.cs

+        private static readonly string TokenEndpoint = ConfigurationManager.AppSettings["TokenEndpoint"] ?? string.Empty;
+        private static readonly string TokenGrantType = ConfigurationManager.AppSettings["grant_type"] ?? string.Empty;
+        private static readonly string TokenClientId = ConfigurationManager.AppSettings["client_id"] ?? string.Empty;
+        private static readonly string TokenClientSecret = ConfigurationManager.AppSettings["client_secret"] ?? string.Empty;
+        private static readonly string TokenResource = ConfigurationManager.AppSettings["FhirEndpoint"] ?? string.Empty;
+        private static readonly string ResourceType = ConfigurationManager.AppSettings["ResourceType"] ?? string.Empty;
+        private static readonly string ContainerName = ConfigurationManager.AppSettings["ContainerName"] ?? string.Empty;
+        private static readonly string ConnectionString = ConfigurationManager.AppSettings["ConnectionString"] ?? string.Empty;
+        private static readonly string FhirEndpoint = TokenResource;
+        private static readonly int NumberOfBlobsForImport = int.Parse(ConfigurationManager.AppSettings["NumberOfBlobsForImport"] ?? "1");
+        private static readonly TimeSpan ImportStatusDelay = TimeSpan.Parse(ConfigurationManager.AppSettings["ImportStatusDelay"]);


When there are this many properties, could we just bind to a configuration object? I think it would simplify all this a lot

brendankowitz · 2023-04-11T17:55:34Z

tools/RegisterAndMonitorImport/RegisterAndMonitorImport.cs

+        private static readonly int NumberOfBlobsForImport = int.Parse(ConfigurationManager.AppSettings["NumberOfBlobsForImport"] ?? "1");
+        private static readonly TimeSpan ImportStatusDelay = TimeSpan.Parse(ConfigurationManager.AppSettings["ImportStatusDelay"]);
+        private static readonly HttpClient HttpClient = new();
+        private static BlobContainerClient s_blobContainerClientSource;


Why are we getting s_ in here? Can we check the right code style is getting applied

The code style is referencing the same file as all other projects.
The IDE will flag warnings or errors as you can see in the screenshot. This is not unique to this project as far as warnings go. There are actually countless examples already that violate this IDE1006 rule if a field is private static and not readonly.

brendankowitz · 2023-04-11T17:57:11Z

tools/RegisterAndMonitorImport/RegisterAndMonitorImport.csproj

+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <AssemblyName>Fhir.RegisterAndMonitorImport</AssemblyName>
+    <RootNamespace>Microsoft.Health.Fhir.RegisterAndMonitorImport</RootNamespace>


Microsoft.Health.Internal...

SergeyGaluzo

* new import tool * revised app.config * Revised based on code review * revisions to handle oss; code cleanup * updated readme to assist oss usage; added config setting for delay when getting import status * revisions from code review * final code review revisions * changes from code review * Adding one additional comment for clarify

v-mserdar added 2 commits March 17, 2023 10:32

new import tool

77776c7

revised app.config

c46774d

v-mserdar added Enhancement Enhancement on existing functionality. Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs labels Mar 17, 2023

v-mserdar added this to the S111 milestone Mar 17, 2023

v-mserdar requested a review from a team as a code owner March 17, 2023 17:48

v-mserdar enabled auto-merge (squash) March 17, 2023 22:02

v-mserdar disabled auto-merge March 17, 2023 22:02

SergeyGaluzo reviewed Mar 20, 2023

View reviewed changes

SergeyGaluzo requested changes Mar 20, 2023

View reviewed changes

v-mserdar added 3 commits March 21, 2023 12:33

Revised based on code review

ab6f61a

revisions to handle oss; code cleanup

b952211

updated readme to assist oss usage; added config setting for delay wh…

495031b

…en getting import status

SergeyGaluzo reviewed Mar 31, 2023

View reviewed changes

revisions from code review

a91df5c

SergeyGaluzo previously approved these changes Mar 31, 2023

View reviewed changes

final code review revisions

a4bc7ea

v-mserdar dismissed SergeyGaluzo’s stale review via a4bc7ea March 31, 2023 22:53

v-mserdar modified the milestones: S111, backlog, Require API Versioning, 112 Mar 31, 2023

SergeyGaluzo previously approved these changes Mar 31, 2023

View reviewed changes

brendankowitz reviewed Apr 11, 2023

View reviewed changes

changes from code review

22a7dbc

v-mserdar dismissed SergeyGaluzo’s stale review via 22a7dbc April 11, 2023 23:31

v-mserdar requested a review from brendankowitz April 18, 2023 20:55

Adding one additional comment for clarify

d346461

SergeyGaluzo approved these changes Apr 18, 2023

View reviewed changes

LTA-Thinking approved these changes Apr 19, 2023

View reviewed changes

v-mserdar merged commit 309a990 into main Apr 21, 2023

v-mserdar deleted the personal/v-mserdar/new_import_tool branch April 21, 2023 03:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new import tool #3182

new import tool #3182

v-mserdar commented Mar 17, 2023 •

edited

Loading

SergeyGaluzo Mar 20, 2023

SergeyGaluzo Mar 20, 2023

v-mserdar Mar 21, 2023 •

edited

Loading

SergeyGaluzo Mar 21, 2023

SergeyGaluzo left a comment

SergeyGaluzo Mar 31, 2023

SergeyGaluzo Mar 31, 2023

SergeyGaluzo Mar 31, 2023

SergeyGaluzo Mar 31, 2023 •

edited

Loading

v-mserdar Mar 31, 2023

SergeyGaluzo Mar 31, 2023

v-mserdar Mar 31, 2023

SergeyGaluzo Mar 31, 2023

v-mserdar Mar 31, 2023

SergeyGaluzo left a comment

SergeyGaluzo left a comment

brendankowitz Apr 11, 2023

brendankowitz Apr 11, 2023

v-mserdar Apr 11, 2023 •

edited

Loading

brendankowitz Apr 11, 2023

SergeyGaluzo left a comment

new import tool #3182

new import tool #3182

Conversation

v-mserdar commented Mar 17, 2023 • edited Loading

Description

Related issues

Testing

FHIR Team Checklist

Semver Change (docs)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

v-mserdar Mar 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyGaluzo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyGaluzo Mar 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyGaluzo left a comment

Choose a reason for hiding this comment

SergeyGaluzo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

v-mserdar Apr 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyGaluzo left a comment

Choose a reason for hiding this comment

v-mserdar commented Mar 17, 2023 •

edited

Loading

v-mserdar Mar 21, 2023 •

edited

Loading

SergeyGaluzo Mar 31, 2023 •

edited

Loading

v-mserdar Apr 11, 2023 •

edited

Loading