PF-3004: Fix stairctl so it runs again, other updates #154

pshapiro4broad · 2024-07-12T20:56:23Z

This PR updates the configuration of stairctl so it works again.

While updating it, I noticed that it makes heavy use of System.out, which isn't the recommended approach for Spring Shell apps. I updated it to use Terminal instead.

While testing, I've used this to look at TDR flight logs in dev and locally, and it seems like it might be useful. It may be useful for other Terra apps that use stairway, for developers, or a tool for support, so they can look at flight failures in more detail without needing to search production logs.

Here's an example session using stairctl:

❯ java -jar stairctl/build/libs/stairctl-1.1.9-SNAPSHOT.jar
StairCtl  (v1.1.9-SNAPSHOT)

[main] INFO bio.terra.stairctl.Application - Starting Application v1.1.9-SNAPSHOT using Java 17.0.4.1 with PID 68497 (/Users/pshapiro/source/stairway/stairctl/build/libs/stairctl-1.1.9-SNAPSHOT.jar started by pshapiro in /Users/pshapiro/source/stairway)
[main] INFO bio.terra.stairctl.Application - No active profile set, falling back to 1 default profile: "default"
[main] INFO bio.terra.stairctl.Application - Started Application in 2.031 seconds (process running for 2.434)
stairctl> connect
Connected to Stairway
Connected to Stairway on database stairwaylib
stairctl> show connection
Stairway Connection:
  dbname  : stairwaylib
  username: stairwayuser
  host    : 127.0.0.1
  port    : 5432
stairctl> list flights

Offset FlightId                             Class                                Submitted                   Completed                   Status       StairwayId                    
------ ------------------------------------ ------------------------------------ --------------------------- --------------------------- ------------ ------------------------------
     0 undoTest                             b.t.s.flights.TestFlightRecoveryUndo 2024-07-12T15:36:39.407627Z 2024-07-12T15:36:44.511316Z ERROR                                      

stairctl> get flight --flightId undoTest

Flight: undoTest
  class     : bio.terra.stairway.flights.TestFlightRecoveryUndo
  submitted : 2024-07-12T15:36:39.407627Z
  completed : 2024-07-12T15:36:44.511316Z
  duration  : 0:00:05.103
  status    : ERROR
  exception : java.lang.RuntimeException;TestStepTriggerUndo
  stairwayId: 
stairctl>

…d to set a property to enable it.

pshapiro4broad · 2024-07-12T20:56:46Z

stairctl/src/main/resources/application.yml

+
+spring:
+  shell:
+    interactive:


Spring shell 3.3 disables the interactive shell by default, so we need to set this property to enable it.

https://github.com/spring-projects/spring-shell/wiki/Spring-Shell-3.3-Release-Notes

pshapiro4broad · 2024-07-12T20:57:35Z

stairctl/build.gradle

+        exclude module: 'spring-boot-starter-logging'
+    }
+
+    implementation 'org.slf4j:slf4j-simple:1.7.36'


This isn't necessary but it cleans up the log configuration errors you get on startup

Fair enough, would you mind adding a comment to that effect on the exclusion?

- use `Terminal` not System.out for output - remove apache utils where possible

pshapiro4broad · 2024-07-26T18:47:11Z

stairctl/src/main/java/bio/terra/stairctl/commands/ConnectCommands.java

@@ -31,47 +33,41 @@ public void connect(
              defaultValue = ShellOption.NULL)
          String dbname,
      @ShellOption(
-              value = {"-h", "--host"},
+              value = {"-H", "--host"},


This was required as -h conflicts with help, which is a global option. Spring shell didn't report an error but it did cause some odd problems when trying to use other options for this command.

okotsopoulos

Thank you for noodling on this! Would you mind also looking at the stairctl readme to see if any updates are needed in light of these changes?

I think that the ticket number also needs to be associated with this PR - https://broadworkbench.atlassian.net/browse/PF-3004

okotsopoulos · 2024-07-30T21:14:14Z

stairctl/build.gradle

+        exclude module: 'spring-boot-starter-logging'
+    }
+
+    implementation 'org.slf4j:slf4j-simple:1.7.36'


Fair enough, would you mind adding a comment to that effect on the exclusion?

okotsopoulos · 2024-07-30T21:14:29Z

stairctl/src/main/java/bio/terra/stairctl/ConnectParams.java

-    this.dbname = dbname;
-    return this;
-  }
+public record ConnectParams(


Nice, love a config-to-record refactor.

okotsopoulos · 2024-07-30T21:14:57Z

stairctl/src/main/java/bio/terra/stairctl/ConnectParams.java

-    dbname = Optional.ofNullable(dbname).orElse(defaults.getDbname());
+  public ConnectParams withDefaults(ConnectParams defaults) {
+    return new ConnectParams(
+        Objects.requireNonNullElseGet(username, defaults::username),


TIL about this Objects helper method.

…changes

- update to latest slf4j. This required converting logback.groovy to logback.xml - change stairctl to not log at all

pshapiro4broad · 2024-08-01T19:52:14Z

buildSrc/src/main/groovy/stairway.java-conventions.gradle

@@ -14,7 +13,7 @@ repositories {

 dependencies {
    // For logging
-    implementation group: 'org.slf4j', name: 'slf4j-api', version: '1.7.36'
+    implementation group: 'org.slf4j', name: 'slf4j-api', version: '2.0.13'


I was able to remove the stairctl subproject dependency exclusion by updating the slf4j version to the current release. This required changing the logback config files to xml, as groovy was deprecated a while ago due to security issues.

pshapiro4broad · 2024-08-01T19:53:16Z

stairctl/src/main/resources/application.yml

+    script:
+      enabled: true
+
+# Disable logging


While looking the logging dependencies, I thought that, as a command line program, stairctl shouldn't log at all, so I added a config to properties to disable logging.

sonarqubecloud · 2024-08-01T19:56:59Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

fboulnois

👍

spring shell 3.3 disables the interactive shell by default, so we nee…

4343236

…d to set a property to enable it.

pshapiro4broad commented Jul 12, 2024

View reviewed changes

misc cleanups / opinionated changes

5e5e189

- use `Terminal` not System.out for output - remove apache utils where possible

pshapiro4broad marked this pull request as ready for review July 26, 2024 13:46

pshapiro4broad requested a review from okotsopoulos July 26, 2024 18:45

pshapiro4broad commented Jul 26, 2024

View reviewed changes

okotsopoulos approved these changes Jul 30, 2024

View reviewed changes

pshapiro4broad changed the title ~~Fix stairctl so it runs again~~ PF-3004: Fix stairctl so it runs again, other updates Aug 1, 2024

pshapiro4broad added 2 commits August 1, 2024 15:03

update docs for --host option name; other misc minor documentation …

b94e8b7

…changes

logging cleanups:

939558d

- update to latest slf4j. This required converting logback.groovy to logback.xml - change stairctl to not log at all

pshapiro4broad commented Aug 1, 2024

View reviewed changes

fboulnois approved these changes Aug 2, 2024

View reviewed changes

pshapiro4broad merged commit baceb1d into develop Aug 5, 2024
3 checks passed

pshapiro4broad deleted the ps/fix-stairctl branch August 5, 2024 18:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PF-3004: Fix stairctl so it runs again, other updates #154

PF-3004: Fix stairctl so it runs again, other updates #154

pshapiro4broad commented Jul 12, 2024 •

edited

Loading

pshapiro4broad Jul 12, 2024 •

edited

Loading

pshapiro4broad Jul 12, 2024

okotsopoulos Jul 30, 2024

pshapiro4broad Jul 26, 2024

okotsopoulos left a comment

okotsopoulos Jul 30, 2024

okotsopoulos Jul 30, 2024

okotsopoulos Jul 30, 2024

pshapiro4broad Aug 1, 2024

pshapiro4broad Aug 1, 2024

sonarqubecloud bot commented Aug 1, 2024

fboulnois left a comment

PF-3004: Fix stairctl so it runs again, other updates #154

PF-3004: Fix stairctl so it runs again, other updates #154

Conversation

pshapiro4broad commented Jul 12, 2024 • edited Loading

pshapiro4broad Jul 12, 2024 • 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

okotsopoulos 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

sonarqubecloud bot commented Aug 1, 2024

Quality Gate passed

fboulnois left a comment

Choose a reason for hiding this comment

pshapiro4broad commented Jul 12, 2024 •

edited

Loading

pshapiro4broad Jul 12, 2024 •

edited

Loading