TSPS-134 Upgrade to Spring Boot 3 and TCL 0.1.9 #61

mmorgantaylor · 2024-03-14T17:25:42Z

Description

This PR upgrades TSPS to Spring Boot 3, which involved a few other changes:

upgrade to terra-common-lib 0.1.9
upgrade other terra service client libraries to jakarta versions
note that the new version of Leo's listAppsV2 endpoint doesn't accept a creatorRoleSpecifier, so we removed it, as we weren't using that functionality anyway
remove spotbugs (following other services, since we use SonarCloud)
convert LeonardoServerConfiguration from a record to a class, because we were getting an error "Non-canonical record constructor must delegate to another constructor" and converting to a class fixed it
update the implementation of EmbeddedDataSource for testing based on this

Jira Ticket

https://broadworkbench.atlassian.net/browse/TSPS-134

handle duplicate job id error add logging for cbas steps - debugging still wip new cbas client more WIP debugging tests WIP WIP working! pre-cleanup some cleanup, re-implement spotbugs trying to get spotbugs to work remove spotbugs! cleanup

jsotobroad

this is great, id love to hear how you fixed the embedded database issues

Also i wonder if we can use the trace stuff in other clients too (to do in another ticket)

jsotobroad · 2024-03-15T03:51:12Z

.github/workflows/build-and-test.yml

@@ -46,11 +46,6 @@ jobs:
      - name: Build the test harness and, by dependency, the service library
        run: ./gradlew --build-cache build -x test

-      - name: Upload spotbugs results


do we remove this because its covered by sonarqube?

yep, a bunch of other services took this out because sonarqube covers these checks. (noted in PR description)

jsotobroad · 2024-03-15T03:52:41Z

buildSrc/src/main/groovy/bio.terra.pipelines.java-common-conventions.gradle

-        languageVersion = JavaLanguageVersion.of(17)
-    }
-}
+// removing the java toolchain gets rid of a warning about using a deprecated API:


do we want to keep this comment w/o the code its referring to? Not sure it'll make sense without it (would probably ok just getting rid of everything).

ah yeah I left this here so we could talk about it more - this is something i'm not sure about

jsotobroad · 2024-03-15T03:53:31Z

client/swagger.gradle

@@ -1,12 +1,19 @@
 dependencies {
+    api 'org.springframework.boot:spring-boot-starter-json'


do we really need spring boot for our client to be created? That seems a bit odd to me.

don't think we do! nice catch

jsotobroad · 2024-03-15T04:14:47Z

service/build.gradle

+    // https://stackoverflow.com/questions/76706612/standard-commons-logging-discovery-in-action-with-spring-jcl
+    // resolves "Standard Commons Logging discovery in action with spring-jcl: please remove commons-logging.jar from classpath in order to avoid potential conflicts"
+    configureEach {
+        exclude group: 'commons-logging', module: 'commons-logging'


would it be worth removing this specifically from each dependency instead of doing it for all our dependencies?

probably yes... i was being lazy because when i took it out of the dependency i expected, it didn't get rid of the warning. i'll work on this

offline conversation: leave as-is for now

jsotobroad · 2024-03-15T04:15:21Z

service/build.gradle

+    implementation "org.broadinstitute.dsde.workbench:sam-client_2.13:0.1-5a5bf28"
+    implementation "org.broadinstitute.dsde.workbench:leonardo-client_2.13:1.3.6-22ee00b"
+    implementation "org.databiosphere:workspacedataservice-client-okhttp-jakarta:0.2.115-20240227.195850-1"
+    implementation "bio.terra:cbas-client:0.0.199"
    implementation "bio.terra:terra-resource-buffer-client:0.4.3-SNAPSHOT"


just noticing this is in here, what is this and does it not need to be updated?

nice catch, we don't use this - I think I added it by mistake when adding Stairway a while ago. will remove

jsotobroad · 2024-03-15T04:18:03Z

service/src/main/java/bio/terra/pipelines/dependencies/sam/SamClient.java

@@ -30,7 +32,7 @@ private ApiClient getApiClient() {
    var okHttpClientWithTracing =
        this.okHttpClient
            .newBuilder()
-            .addInterceptor(new OkHttpClientTracingInterceptor(Tracing.getTracer()))
+            .addInterceptor(new OkHttpClientTracingInterceptor(openTelemetry))


can we do this for wds as well? i believe they also use okHttp to make their client

definitely we can - are you ok if that goes in the tracing ticket (TSPS-107) rather than this PR?

jsotobroad · 2024-03-15T04:18:40Z

service/src/main/java/bio/terra/pipelines/dependencies/stairway/JobService.java

            .exceptionSerializer(new StairwayExceptionSerializer(objectMapper)));
  }

  /** Retrieves Job Result specifying the result class type. */
-  @Traced
+  @WithSpan


are there any docs we can link to like in the readme to explain this tracing stuff?

probably Doug's blog post (which is linked in the tracing ticket), or stuff that's linked in it. do you want me to add that documentation in this PR?

jsotobroad · 2024-03-15T04:19:09Z

service/src/main/resources/application.yml

@@ -61,6 +66,13 @@ spring:
    url: ${env.db.host}/${env.db.pipelines.name}
    username: ${env.db.pipelines.user}

+  jpa:
+    # set the following to true to enable SQL logging in JPA
+    show-sql: false


awesome thanks for the debugging switches!

jsotobroad

nice this is awesome!!

sonarqubecloud · 2024-03-15T15:36:30Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

mmorgantaylor marked this pull request as ready for review March 14, 2024 19:08

mmorgantaylor requested a review from jsotobroad March 14, 2024 19:08

WIP

5a4277e

handle duplicate job id error add logging for cbas steps - debugging still wip new cbas client more WIP debugging tests WIP WIP working! pre-cleanup some cleanup, re-implement spotbugs trying to get spotbugs to work remove spotbugs! cleanup

mmorgantaylor force-pushed the TSPS-134_mma_sb3_round2 branch from db45338 to 5a4277e Compare March 14, 2024 19:21

ensure stairway doesn't try to initialize in tests

b725107

jsotobroad reviewed Mar 15, 2024

View reviewed changes

mmorgantaylor added 3 commits March 15, 2024 08:39

PR review updates 1

8ef7d9e

PR review updates 2

1be24ad

add to README

a58261b

jsotobroad approved these changes Mar 15, 2024

View reviewed changes

cleanup

7777974

mmorgantaylor merged commit eb070a9 into main Mar 15, 2024
12 checks passed

mmorgantaylor deleted the TSPS-134_mma_sb3_round2 branch March 15, 2024 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TSPS-134 Upgrade to Spring Boot 3 and TCL 0.1.9 #61

TSPS-134 Upgrade to Spring Boot 3 and TCL 0.1.9 #61

mmorgantaylor commented Mar 14, 2024 •

edited

Loading

jsotobroad left a comment

jsotobroad Mar 15, 2024

mmorgantaylor Mar 15, 2024

jsotobroad Mar 15, 2024

mmorgantaylor Mar 15, 2024

jsotobroad Mar 15, 2024

mmorgantaylor Mar 15, 2024

jsotobroad Mar 15, 2024

mmorgantaylor Mar 15, 2024

mmorgantaylor Mar 15, 2024

jsotobroad Mar 15, 2024

mmorgantaylor Mar 15, 2024

jsotobroad Mar 15, 2024

mmorgantaylor Mar 15, 2024

jsotobroad Mar 15, 2024

mmorgantaylor Mar 15, 2024

jsotobroad Mar 15, 2024

jsotobroad left a comment

sonarqubecloud bot commented Mar 15, 2024

		@@ -1,12 +1,19 @@
		dependencies {
		api 'org.springframework.boot:spring-boot-starter-json'

TSPS-134 Upgrade to Spring Boot 3 and TCL 0.1.9 #61

TSPS-134 Upgrade to Spring Boot 3 and TCL 0.1.9 #61

Conversation

mmorgantaylor commented Mar 14, 2024 • edited Loading

Description

Jira Ticket

jsotobroad 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsotobroad left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 15, 2024

Quality Gate passed

mmorgantaylor commented Mar 14, 2024 •

edited

Loading