Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add shadow jar and workspace controller that starts said jar #726

Merged
merged 4 commits into from
Apr 10, 2024

Conversation

MrCreosote
Copy link
Member

Tested this setup in the Groups service, replacing the workspace controller there

Next step is to port the workspace controller from groups to use the
shadow jar in a similar manner to auth, negating the need for the jars
repo or a jars list
Tested in groups with the shadow jar. Needed some minor test changes but
passed.
@@ -267,10 +294,9 @@ dependencies {
)
testImplementation 'com.arangodb:arangodb-java-driver:6.7.2'
testImplementation 'com.github.zafarkhaja:java-semver:0.9.0'
testImplementation 'org.hamcrest:hamcrest-core:1.3'
testImplementation 'commons-lang:commons-lang:2.4'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out this isn't needed

Copy link

codecov bot commented Apr 4, 2024

Codecov Report

Merging #726 (2c17333) into dev-gradle2 (5d9b354) will not change coverage.
Report is 3 commits behind head on dev-gradle2.
The diff coverage is n/a.

❗ Current head 2c17333 differs from pull request most recent head ed37ddc. Consider uploading reports for the commit ed37ddc to get more accurate results

Additional details and impacted files
@@              Coverage Diff               @@
##             dev-gradle2     #726   +/-   ##
==============================================
  Coverage          87.74%   87.74%           
  Complexity          5330     5330           
==============================================
  Files                228      228           
  Lines              17635    17635           
  Branches            2569     2569           
==============================================
  Hits               15473    15473           
  Misses              1700     1700           
  Partials             462      462           

build.gradle Show resolved Hide resolved
Copy link
Collaborator

@Xiangs18 Xiangs18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
@ialarmedalien should also take a look!

Copy link
Member

@briehl briehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotta be honest, I'm not entirely sure how to review here. I looked at the shadow jar docs (https://imperceptiblethoughts.com/shadow/) and the gradle stuff looks fine to me.

So here's the summary if I'm reading it right.

build.gradle's updated to make a shadow jar - a single jar containing the whole workspace service + docs + the new WorkspaceController.

WorkspaceController can then be invoked to stand up a temporary Workspace service to... do stuff? Does it run its own tests? Does that shadow jar get included with other Java-based services to run a temporary workspace a little more seamlessly than running, say, a Docker image?

Anyway, that looks like (from some other test module) you can just go:

final WorkspaceController ws = new WorkspaceController(params);
... do test stuff ...
ws.destroy();

which is kinda awesome.

Anyway, if that's the intent, this all seems to hit all the buttons and link together. Hitting approve, but there's a typo and a question there, at your leisure.

build.gradle Outdated
var BUILD_JAVA_DOC_DIR = "$BUILD_DOC_ROOT/javadoc"
var BUILD_OTHER_DOC_DIR = "$BUILD_DOC_ROOT/otherdoc/"
// This is where the DocServer will look for docs, if it changes it needs to change
// in deloy.cfg as well
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: deploy.cfg

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed


workspace = servpb.start();
// TODO TEST add periodic check w/ exponential backoff
Thread.sleep(5000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just to make sure that connections get made properly? Are there asynchronous things (like servpb.start()) that go off in this time that are more of a pain to monitor?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workspace takes a second or two to start up after ProcessBuilder.start() is called to check all the connections to other services. This just waits until it should be ready to accept connections.

Copy link
Member Author

@MrCreosote MrCreosote Apr 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could do a microoptimization by calling workspace.ver() in a loop waiting for it to come up as per the TODO but it doesn't really seem worth it for now

@MrCreosote
Copy link
Member Author

MrCreosote commented Apr 10, 2024

WorkspaceController can then be invoked to stand up a temporary Workspace service to... do stuff? Does it run its own tests? Does that shadow jar get included with other Java-based services to run a temporary workspace a little more seamlessly than running, say, a Docker image?

It's for cases like this: https://github.com/kbase/groups/blob/master/src/us/kbase/test/groups/integration/ServiceIntegrationTest.java#L124-L132

With the shadowjar, you no longer need to list all the dependent jars: https://github.com/kbase/groups/blob/master/src/us/kbase/test/groups/controllers/workspace/wsjars

So you just add that one jar to your classpath and can easily start up a workspace to test against.

Eventually I want the workspace repo to publish the shadow jar via jitpack but that's a bit farther down the road

@MrCreosote
Copy link
Member Author

which is kinda awesome.

and it's totally awesome, thank you very much

@MrCreosote MrCreosote merged commit 602fbdb into kbase:dev-gradle2 Apr 10, 2024
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants