-
Notifications
You must be signed in to change notification settings - Fork 18
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
[Client] Java client #198
[Client] Java client #198
Conversation
448fb73
to
58b53c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we keep the two clients separately in different projects
# https://docs.gradle.org/current/userguide/platforms.html#sub::toml-dependencies-format | ||
|
||
[versions] | ||
guava = "33.1.0-jre" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we import guava? Do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, let me check.
# JAVA Client | ||
|
||
## Prerequisites | ||
|
||
- Java JDK | ||
- Gradle 7.0 or later | ||
|
||
## Building the Project | ||
|
||
To build the project, run: | ||
``` | ||
./gradlew build | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add documentation on how to add this to a separate project?
guava = "33.1.0-jre" | ||
junit-jupiter = "5.10.2" | ||
|
||
[libraries] | ||
guava = { module = "com.google.guava:guava", version.ref = "guava" } | ||
junit-jupiter = { module = "org.junit.jupiter:junit-jupiter", version.ref = "junit-jupiter" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need these libraries?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will check
public static RustLib rustLib; | ||
|
||
public CacClient(String libraryPath, String libraryName) { | ||
System.setProperty("jnr.ffi.library.path", libraryPath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read library path from ENV
SUPERPOSITION_LIB_PATH
int result = rustLib.cac_new_client(tenant, updateFrequency, hostName); | ||
if (result > 0) { | ||
String errorMessage = rustLib.cac_last_error_message(); | ||
throw new IOException("Failed to create new CAC client: " + errorMessage); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not an IOException right? Can we create a special Exception object for this?
if (ptr == null) { | ||
return null; | ||
} | ||
try { | ||
return ptr.getString(0); | ||
} finally { | ||
freeString(ptr); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we write this better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't it looks fine?
private static void callCacClient() { | ||
String dylib = "cac_client"; | ||
File currentDir = new File(System.getProperty("user.dir")); | ||
String libraryPath = currentDir.getParentFile().getParentFile().getParentFile() + "/target/debug"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No hardcoding the path please, use env SUPERPOSITION_LIB_PATH
|
||
int newClient; | ||
try { | ||
newClient = wrapper.cacNewClient(tenant, 1, "http://localhost:8080"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The URL should not be hardcoded, is this an example file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is an example file.
|
||
import jnr.ffi.Pointer; | ||
|
||
public class Client { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we name this class better? It seems like a Client implementation
@@ -0,0 +1,116 @@ | |||
package CAC; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Package should be different
Can you add java and gradle versions in flakes.nix as well? |
65d78a4
to
9df9c7f
Compare
6ca353c
to
41073fc
Compare
fb9f0ad
to
7c87aa6
Compare
35e61af
to
fd5c309
Compare
1355e99
to
a8c218d
Compare
469448f
to
f24abe8
Compare
4ac0f1a
to
2599c16
Compare
2599c16
to
6a73495
Compare
* feat: java client * fix: Removed unnecessary files * fix: more cleanups * feat: Add expt client wrapper * fix: add experiment client implementation * fix: Added README.md * fix: separated cac-client and exp-client * fix: separated cac-client and exp-client * fix: make it a lib * fix: Added Readme.md to use the clients * fix: yaml file --------- Co-authored-by: Ankit Mahato <[email protected]>
Problem
Java CAC Client