Refresh IG Group file POST addition & messaging improvements #506

echicoine-icf · 2023-12-14T20:13:09Z

This pull request covers the following changes:

Refresh IG POST expanded to include other resources such as group files.
Duplicate bundle files during refresh fixed.
added -x as possible argument during Refresh IG to show expanded reporting for errors, warnings, and information messages.
- Boolean "verboseMessaging" can be found throughout methods and when true, extra information is appended to output such as cql translation.
Bundle Test Cases enhanced with multithreading optimizations.
General enhancements to message output for readability.
Renamed AbstractResourceProcessor to AbstractBundler.
- Decoupled MeasureProcessor from AbstractBundler, extends BaseProcessor
- Created MeasureBundler, extends AbstractBundler
- Renamed PlanDefinitionProcessor to PlanDefinitionBundler
- Renamed QuestionnaireProcessor to QuestionnaireBundler
Improved readability and complexity behind IGBundleProcessor instantiation.
Adjusted CqlTranslatorException to use List
-Cql translation results adjusted for output such as:

Github Issue:
I've read the contribution guidelines
Code compiles without errors
Tests are created / updated
Documentation is created / updated

By creating this PR you acknowledge that your contribution will be licensed under Apache 2.0

Duplicate bundle files during refresh fixed. -x added as possible argument during Refresh IG to show expanded reporting for errors, warnings, and information messages. Bundle Test Cases enhanced with multithreading optimizations. General enhancements to message output for readability.

…o avoid possible duplicate keys

…rious methods during bundling process, to relay exception messages to uusers

…chicoine-icf/cqf-tooling into refresh-messaging-improvements

…ummary during refresh.

… abstract class. Finished message won't appear if no resources exist for bundling.

…and can cleanly report to measure developers the issues they have with tests.

…l post. Added ability to ignore certain tests during refresh.

…sage generation to its own method.

…r so that warnings from ca.uhn.fhir.parser.LenientErrorHandler can be supressed during various operations. We already handle the translator error list.

ddieppois

Small changes regarding use of log/Sysout. Very interesting to see a massive refactoring and the new functionalities being more explained.

ddieppois · 2024-01-29T17:18:04Z

tooling/src/main/java/org/opencds/cqf/tooling/utilities/ResourceUtils.java

+         outputResourceTracker.put(resourceFileLocation, Boolean.TRUE);
+
+      } catch (IOException e) {
+         e.printStackTrace();


This could be a log.error

ddieppois · 2024-01-29T17:18:19Z

tooling/src/main/java/org/opencds/cqf/tooling/utilities/ResourceUtils.java

+         );
+         writer.flush();
+      } catch (IOException e) {
+         e.printStackTrace();


Same here "log.error"

ddieppois · 2024-01-29T17:23:38Z

tooling-cli/pom.xml

@@ -32,12 +40,6 @@
            <artifactId>model-jackson</artifactId>
        </dependency>

-        <dependency>


Could we apply this change on the main pom.xml and do a refactoring of logger to complete this ticket : #333 @JPercival ?

ddieppois · 2024-01-29T17:31:10Z

tooling/src/main/java/org/opencds/cqf/tooling/library/LibraryProcessor.java

@@ -76,8 +78,7 @@ public List<String> refreshIgLibraryContent(BaseProcessor parentContext, Encodin
    }

    public List<String> refreshIgLibraryContent(BaseProcessor parentContext, Encoding outputEncoding, String libraryPath, String libraryOutputDirectory, Boolean versioned, FhirContext fhirContext, Boolean shouldApplySoftwareSystemStamp) {
-        System.out.println("Refreshing libraries...");
-        // ArrayList<String> refreshedLibraryNames = new ArrayList<String>();
+        System.out.println("\r\n[Refreshing Libraries]\r\n");


It might be better to use logger here

ddieppois · 2024-01-29T17:34:12Z

tooling/src/main/java/org/opencds/cqf/tooling/measure/MeasureProcessor.java


-        logger.info("Refreshing measures...");
+        System.out.println("\r\n[Refreshing Measures]\r\n");


logger might be better here.

ddieppois · 2024-01-29T17:36:56Z

tooling/src/main/java/org/opencds/cqf/tooling/processor/CqlProcessor.java

@@ -250,7 +249,7 @@ public LibraryManager getLibraryManager() {
    }

    private void translateFolder(String folder) {
-        logger.logMessage(String.format("Translating CQL source in folder %s", folder));
+        System.out.printf("Translating CQL source in folder %s%n", folder);


Same here for logger with a String format that uses "folder"

ddieppois · 2024-01-29T17:37:23Z

tooling/src/main/java/org/opencds/cqf/tooling/processor/CqlProcessor.java

@@ -343,7 +342,7 @@ public static ValidationMessage exceptionToValidationMessage(File file, CqlCompi
    }

    private void translateFile(LibraryManager libraryManager, File file, CqlCompilerOptions options) {
-        logger.logMessage(String.format("Translating CQL source in file %s", file.toString()));
+//        logger.logMessage(String.format("Translating CQL source in file %s", file.toString()));


Duplicated line

ddieppois · 2024-01-29T17:37:57Z

tooling/src/main/java/org/opencds/cqf/tooling/processor/CqlProcessor.java

                } catch (Exception ex) {
                    logger.logMessage(String.format("CQL Translation succeeded for file: '%s', but ELM generation failed with the following error: %s", file.getAbsolutePath(), ex.getMessage()));
                }
            }
+
+            //output Success/Warn/Info/Fail message to user:
+            System.out.println(buildStatusMessage(translator.getErrors(), file.getName(), verboseMessaging));


logger might be better here

ddieppois · 2024-01-29T17:39:15Z

tooling/src/main/java/org/opencds/cqf/tooling/processor/IGBundleProcessor.java


        //run collected post calls last:
        if (HttpClientUtils.hasPostTasksInQueue()) {
-            System.out.println("\r\n[POST task(s) found in queue. POST task(s) started - " + getTime() + "]");
+            System.out.println("\r\n[Persisting Files to " + fhirUri + "]\r\n");


suggesting use of logger here instead of System.out.println

ddieppois · 2024-01-29T17:41:46Z

tooling/src/main/java/org/opencds/cqf/tooling/processor/IGTestProcessor.java

@@ -172,105 +172,109 @@ public void testIg(TestIGParameters params) {

        CqfmSoftwareSystem testTargetSoftwareSystem =  getCqfRulerSoftwareSystem(params.fhirServerUri);

-        logger.info("Running IG test cases...");
+        System.out.println("\r\n[Running IG Test Cases]\r\n");


Suggesting use of logger here for this file

The decision to use plain System.out.println came from trying to keep the console readable for the users. Logger includes a big time stamp for every entry and can clutter the console window while we try to convey useful information to the user as it all flies by.

If that sort of thing isn't as necessary as I previously thought I can revert these to use logger.

The log format can be configured, and it's possible to simultaneously have different formats for different targets. For example, simple output that's redirected to the CLI output, and verbose output/formatting going to a log file. That is a separate level of configuration that's needed, but routing messaging through the logger API allows that type of configuration.

Sounds good to me! I'll go ahead and adjust this to use the logger and we can revisit cleaning up the format

echicoine-icf · 2024-03-01T19:18:43Z

Logger is now used over System.out

codecov · 2024-03-01T19:22:37Z

Codecov Report

Attention: Patch coverage is 52.98246% with 402 lines in your changes are missing coverage. Please review.

Project coverage is 20.99%. Comparing base (d395b04) to head (69091a7).

❗ Current head 69091a7 differs from pull request most recent head 3999567. Consider uploading reports for the commit 3999567 to get more accurate results

Files	Patch %	Lines
...opencds/cqf/tooling/processor/AbstractBundler.java	58.05%	72 Missing and 27 partials ⚠️
...encds/cqf/tooling/processor/TestCaseProcessor.java	46.82%	58 Missing and 9 partials ⚠️
...opencds/cqf/tooling/utilities/HttpClientUtils.java	45.00%	63 Missing and 3 partials ⚠️
...opencds/cqf/tooling/processor/IGTestProcessor.java	0.00%	54 Missing ⚠️
...g/opencds/cqf/tooling/utilities/ResourceUtils.java	62.00%	16 Missing and 3 partials ⚠️
.../tooling/cql/exception/CqlTranslatorException.java	0.00%	16 Missing ⚠️
...va/org/opencds/cqf/tooling/common/ThreadUtils.java	45.83%	11 Missing and 2 partials ⚠️
...f/tooling/processor/PostBundlesInDirProcessor.java	0.00%	12 Missing ⚠️
...org/opencds/cqf/tooling/processor/IGProcessor.java	66.66%	9 Missing and 2 partials ⚠️
...rg/opencds/cqf/tooling/measure/MeasureBundler.java	75.60%	5 Missing and 5 partials ⚠️
... and 10 more

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #506      +/-   ##
============================================
+ Coverage     20.85%   20.99%   +0.13%     
- Complexity     1494     1520      +26     
============================================
  Files           282      283       +1     
  Lines         24090    24300     +210     
  Branches       3832     3867      +35     
============================================
+ Hits           5024     5101      +77     
- Misses        18221    18351     +130     
- Partials        845      848       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bryantaustin13

LGTM

Evan Chicoine added 7 commits December 13, 2023 20:01

Tracking POST tasks via IBaseResource as key rather than resourceID t…

93c4982

…o avoid possible duplicate keys

Corrected formatting of cql processing summary.

1f6aa65

Code cleanup.

ed0acb4

users

cbff38d

Adjusted over to exception handling rather than boolean return for va…

03539eb

…rious methods during bundling process, to relay exception messages to uusers

Merge branch 'refresh-messaging-improvements' of https://github.com/e…

ee2e580

…chicoine-icf/cqf-tooling into refresh-messaging-improvements

echicoine-icf changed the title ~~Refresh messaging improvements~~ Refresh IG Group file POST addition Dec 14, 2023

Evan Chicoine and others added 14 commits December 18, 2023 13:00

Safeguarding lists against concurrency issues.

62b356a

Adjusted some Maps in IOUtils to be concurrent safe, clarified POST s…

0278789

…ummary during refresh.

Moved start/end messages for bundle process from igbundleprocessor to…

0825b15

… abstract class. Finished message won't appear if no resources exist for bundling.

Consistent titling of sections in console (ie [Bundling Measures]

ac43428

Group file creation bug resolved.

d9570c8

Added Group file verification during Refresh IG test.

f352f9a

Added failed test tracking so we don't have to delete any test files …

e22f9ca

…and can cleanly report to measure developers the issues they have with tests.

Moving some hard coded strings to constants

5a44f38

Added failed POST logging so users can retain a log of what files fai…

b7ef5f9

…l post. Added ability to ignore certain tests during refresh.

Sorting all summary lists at end of bundle process. Moved summary mes…

9321b0f

…sage generation to its own method.

applying a timestamp to the httpfail log filename

7df3885

Sorting the test case refresh summary for readability.

21cd226

Using SLF4J provider ch.qos.logback.classic.spi.LogbackServiceProvide…

cb6ef87

…r so that warnings from ca.uhn.fhir.parser.LenientErrorHandler can be supressed during various operations. We already handle the translator error list.

Merge branch 'master' into refresh-messaging-improvements

53ec57a

JPercival requested review from ddieppois and JPercival January 26, 2024 17:15

ddieppois approved these changes Jan 29, 2024

View reviewed changes

JPercival and others added 2 commits March 1, 2024 10:03

Merge branch 'master' into refresh-messaging-improvements

cd9af26

Using logger over System.out

69091a7

echicoine-icf changed the title ~~Refresh IG Group file POST addition~~ Refresh IG Group file POST addition & messaging improvements Mar 1, 2024

Merge branch 'master' into refresh-messaging-improvements

3999567

bryantaustin13 self-assigned this Mar 18, 2024

bryantaustin13 self-requested a review March 18, 2024 20:23

JPercival enabled auto-merge (squash) March 18, 2024 20:23

bryantaustin13 approved these changes Mar 18, 2024

View reviewed changes

JPercival merged commit dca1a3d into cqframework:master Mar 18, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refresh IG Group file POST addition & messaging improvements #506

Refresh IG Group file POST addition & messaging improvements #506

echicoine-icf commented Dec 14, 2023

ddieppois left a comment

ddieppois Jan 29, 2024

ddieppois Jan 29, 2024

ddieppois Jan 29, 2024

ddieppois Jan 29, 2024

ddieppois Jan 29, 2024

ddieppois Jan 29, 2024

ddieppois Jan 29, 2024

ddieppois Jan 29, 2024

ddieppois Jan 29, 2024

ddieppois Jan 29, 2024

echicoine-icf Feb 12, 2024

JPercival Mar 1, 2024

echicoine-icf Mar 1, 2024

echicoine-icf commented Mar 1, 2024

codecov bot commented Mar 1, 2024 •

edited

Loading

bryantaustin13 left a comment


		logger.info("Refreshing measures...");
		System.out.println("\r\n[Refreshing Measures]\r\n");

Refresh IG Group file POST addition & messaging improvements #506

Refresh IG Group file POST addition & messaging improvements #506

Conversation

echicoine-icf commented Dec 14, 2023

ddieppois 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

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

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

echicoine-icf commented Mar 1, 2024

codecov bot commented Mar 1, 2024 • edited Loading

Codecov Report

bryantaustin13 left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 1, 2024 •

edited

Loading