the current codebase has some ugly parts
#JShell project for Tj-Bot!
timber mirage
should be refactored ig
not before tests
bright lark
not before tests
alr we can do some mvc tests to make sure endpoints work as expected
some unit tests to keep some logic safe, like the id validation
not the time to integration test
timber mirage
The problem is that any more rework/improvment may break stuff, so we need tests to be sure everything is alright
timber mirage
not the time to integration test
well that's the problem
id validation is like the least important test
we need to test if the sessions work
because that's the thing that can break
timber mirage
We need them
Because otherwise there is no way to know if we didn't break something
That's the next priority after the bug fix
timber mirage
Yea that's a problem
as long as we don't have a way to test the endpoints automatically
doing a refactor is too dangerous
Note that spring allows to do unit with doing requests to endpoints
so it might be easier than we think
bright lark
Note that spring allows to do unit with doing requests to endpoints
u mean without* ?
timber mirage
bright lark
if we test endpoints, we'll need to mock deps
timber mirage
me after reading JShellService class
nooooo
compact holly
me after reading JShellService class
don't be a meanie :(
it's not constructive
instead, suggest improvements 
and if they make sense after discussing with us, go implement 'em!
bright lark
im gonna share something in few mins
stay tunned
i still dnt know why id comes from client
it could lead to duplicates
nevermind
what do u think of this ?
sterile crest
beautiful
bright lark
the project is called JShellPlaygroundBackend
somebody may ask backend for what ?
here's finished version
timber mirage
the project is called JShellPlayground**Backend**
yes ?
it doesn't matter
the bot is out of the scope of this project
timber mirage
You could improve both text and simplify them
Also it doesn't really fetch anything, it just send the code to execute
bright lark
but u said it could retrieve snippets ?
timber mirage
it could
bright lark
the diagram has to show all major aspects
timber mirage
but there is no notion to retrieve a session, it doesn't make sense
bright lark
ah
timber mirage
yes
bright lark
and it could run 2 or 3
timber mirage
what are 2 or 3 ?
bright lark
containers
timber mirage
it will select the correct one by name
timber mirage
but it's possible to have multiple wrapper containers
yes?
bright lark
so i make sense to show 2 containers on diag
bright lark
you shouldn't diagram whilst not knowing the codebase
the code is complicated and i think it has serious issues
timber mirage
yes
bright lark
alr
sterile crest
the code is complicated and i think it has serious issues
its not though
sterile crest
if you find parts of the code complicated, ask for clarification
me and ala both understand it and can follow it very well
bright lark
me and ala both understand it and can follow it very well
cuz u ve been on it since long time
but that wont help newcomers
bright lark
i jumped on in less than a day
so u think everything is fine
timber mirage
cuz u ve been on it since long time
Saying the code is complicated is an overstatement
I have seen much more complex codebase
This is nothing
sterile crest
i didn't say that
bright lark
Saying the code is complicated is an overstatement
I have seen much more complex...
me too
sterile crest
what do you find complicated about the jshellsessionservice @bright lark ?
bright lark
what do you find complicated about the jshellsessionservice <@357507852560236546...
JShellService
sterile crest
what part about it is complicated?
sterile crest
public JShellService(DockerService dockerService,
JShellSessionService sessionService,
String id,
long timeout,
boolean renewable,
long evalTimeout,
long evalTimeoutValidationLeeway,
int sysOutCharLimit,
int maxMemory,
double cpus,
String cpuSetCpus,
String startupScript)
so these are parameters that the docker client needs
and some of them are for us
so if we passed a "DockerConfig" instead, is the class now no longer complicated?
the same problem will happen, it'll be a complex record, builder, factory, whatever you decide
but it's all loaded for us
timber mirage
so if we passed a "DockerConfig" instead, is the class now no longer complicated...
I already have a local branch ready for this btw
I am just waiting for the tests before I try to create a pr
sterile crest
you don't need to use the constructor yourself
it's just loaded from that config file
bright lark
you don't need to use the constructor yourself
i need to be able to read it
sterile crest
the actual complicated part of the constructor
is not the parameters
try {
String containerId = dockerService.spawnContainer(maxMemory, (long) Math.ceil(cpus),
cpuSetCpus, containerName(), Duration.ofSeconds(evalTimeout), sysOutCharLimit);
PipedInputStream containerInput = new PipedInputStream();
this.writer = new BufferedWriter(
new OutputStreamWriter(new PipedOutputStream(containerInput)));
InputStream containerOutput =
dockerService.startAndAttachToContainer(containerId, containerInput);
reader = new BufferedReader(new InputStreamReader(containerOutput));
writer.write(sanitize(startupScript));
writer.newLine();
writer.flush();
checkContainerOK();
startupScriptSize = Integer.parseInt(reader.readLine());
} catch (Exception e) {
LOGGER.warn("Unexpected error during creation.", e);
markAsDead();
throw new DockerException("Creation of the session failed.", e);
}
it's this
bright lark
is not the parameters
i didn't say tht
timber mirage
```java
try {
String containerId = dockerService.spawnContai...
For my defense, I wasn't the one who wrote this 👀
sterile crest
that's okay, i already blame istan
bright lark
it should be moved in a service class
sterile crest
JShellService
timber mirage
it should be moved in a service class
it's already in the service class
bright lark
JShell**Service**
nah, it's a model class
sterile crest
why does it need an id?
timber mirage
nah, it's a model class
well
not really
controller -> JShellSessionService -> JShellService -> DockerService
Here we are in the 3rd one
the left has a dependency to the right
in this order
sterile crest
the problem is that JShellSessionService and JShellService are coupled
sterile crest
if we can remove that and handle it in the controller it'll be a little neater
we don't need a model
the only model we need is for Session's
and that can branch out of it needed
bright lark
the only model we need is for Session's
sessions are JShellService
thts what the code says
sterile crest
but JShellService is used to run the command commands??
it's got methods like eval
readResult
timber mirage
the problem is that JShellSessionService and JShellService are coupled
I already have a fix for that too, but it will wait for later, not a problem currently
bright lark
thts what the code says
private synchronized JShellService createSession(SessionInfo sessionInfo) {}
createSession returns JShellService
bright lark
there is no class with tht name
sterile crest
ik
bright lark
actually i feel super confused between these 2
JShellService and JShellSessionService
timber mirage
JShellService and JShellSessionService
JShellSessionService handles sessions
JShellService is one session
sterile crest
if we used game driven development, it would be pretty cool actually, the JShellSessionService could be like an "action handler" and we can use a "game loop" to handle cleanups, changing states etc.
and players == JShellSession's
the code becomes easier to understand as humans
bright lark
trust me, it's not readable (these 2 core ~~service ~~classes)
timber mirage
Ah and also
sterile crest
i trust you, that you find it not readable
timber mirage
for the reason why JShellService isn't called "session"
it's because JShellService has no knowledge of being a session, it's JShellSessionService which make it a session
timber mirage
trust me, it's not readable (these 2 core ~~service ~~classes)
they have logic, they can't be model
sterile crest
yeah so insense it's indirectly like: java class JShellSession { JShellSession(JShellService service) {} }
even though that's not the literal code
you can model the ideas in your head and visually in your mind see the pattern being used
even if not explict in the code
bright lark
they have logic, they can't be model
i know, but they could be refactored
sterile crest
maybe you read code differently, but if you can associate the design with somebody familar to you, you'll understand it
bright lark
so they become : 1 service class + 1 model class
bright lark
🤷♂️
sterile crest
but not yet
bright lark
lmao
timber mirage
and also it doesn't make it easier to read
sterile crest
you can't begin to refactor something you don't understand
timber mirage
like what do you want to refactor ? the bazillion ? parameters in JShellService ?
It won't change much in term of readability
sterile crest
which is where stuff like UML would help
if we had all the classes represented on a diagram
we can visually pen and paper refactor
timber mirage
which is where stuff like UML would help
You can create a class diagram in intellij btw
sterile crest
and all come to an agreement
sterile crest
You can create a class diagram in intellij btw
yeah but it sucks ass
sterile crest
i don't think that's useful for the problems we've had
the tests you've got so far are more around eval
and docker tests are not easy
timber mirage
@bright lark I would like that you do them, is it possible ?
You do not need to understand the code, just think of it as a black box testing, just know how to use the api
timber mirage
and docker tests are not easy
yes but that's the problem
sterile crest
yeah maybe firas could do it
timber mirage
The thing that has the more chances to fail is docker
sterile crest
it would deffo help him a lot
as well help with gather confidence in jshell
but honestly, starting again from scratch would be the best approach instead of pick and choose refactoring (just imo)
we can start re-implementing the code just in a more managed way
once we fully break down the system
timber mirage
I don't find a problem with the system
sterile crest
which is fine if the community can maintain it
we should make it a more TJ project rather than an Ala project
but it's unapproachable because of the lack of understanding people have, it's more harder because it's a new project
unlike TJ Bot which is at least based on common tooling (JDA)
but the mess we're in now is because of improper planning which was unavoidable anyway
bright lark
<@357507852560236546> I would like that you do them, is it possible ?
You do not...
u mean integration test ?
sterile crest
yes @bright lark those are the most beneficial atm
bright lark
yes <@357507852560236546> those are the most beneficial atm
i dnt see this feasible
sterile crest
why?
sterile crest
what complexity do you see?
i already have a good solution in mind
we have all the methods needed to help with assertions
bright lark
thoses injections
timber mirage
@sterile crest https://spring.io/guides/gs/testing-web
sterile crest
^^
I was just about to mention
we use Spring and it has all the tooling available
bright lark
it's not about tooling
sterile crest
then?
bright lark
it's about a not necessary complexity
sterile crest
what complexity though
sterile crest
how does this answer the question?
bright lark
(i didn't fully read docker service yet)
bright lark
how does this answer the question?
what question
sterile crest
what complexity..
timber mirage
(i didn't fully read docker service yet)
Don't read it, your eyes will bleed
@bright lark you just need to do black box testing, this is unrelated to anything you said
bright lark
<@357507852560236546> you just need to do black box testing, this is unrelated t...
never herd of this type
sterile crest
it should be straight forward : ```
@BeforeAll
void startSpring() {}
@Test
void someTest() {
// either post to the API
// call the JShellService directly
// something else
assert(...);
}
It's when you don't assume the implementation, you only look at the input/output
bright lark
tht means mocking
sterile crest
no it doesn't
timber mirage
it should be straight forward : ```
@BeforeAll
void startSpring() {}
@Test
void...
That's not how spring works but yes
bright lark
spring is flexible, but i assume in integration tests, i need to load a spring context with required beans, like the app does in reality
sterile crest
@SpringBootTest
class SmokeTest {
@Autowired
private HomeController controller;
@Test
void contextLoads() throws Exception {
assertThat(controller).isNotNull();
}
}
so this example let's you do your spring magic
assuming the server is running:
@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)
class HttpRequestTest {
@LocalServerPort
private int port;
@Autowired
private TestRestTemplate restTemplate;
@Test
void greetingShouldReturnDefaultMessage() throws Exception {
assertThat(this.restTemplate.getForObject("http://localhost:" + port + "/",
String.class)).contains("Hello, World");
}
}
i know how to test in spring
timber mirage
```java
@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)
class HttpR...
this is a better example
bright lark
it's not going to be like this
after context is loaded for tests, we need to pull img
then perform api call to run a container
sterile crest
it happens under the hood
let me check but i'm sure the code pulls the image if not present
public String spawnContainer(long maxMemoryMegs, long cpus, @Nullable String cpuSetCpus,
String name, Duration evalTimeout, long sysoutLimit) throws InterruptedException {
String imageName = "togetherjava.org:5001/togetherjava/jshellwrapper";
boolean presentLocally = client.listImagesCmd()
.withFilter("reference", List.of(imageName))
.exec()
.stream()
.flatMap(it -> Arrays.stream(it.getRepoTags()))
.anyMatch(it -> it.endsWith(":master"));
if (!presentLocally) {
client.pullImageCmd(imageName)
.withTag("master")
.exec(new PullImageResultCallback())
.awaitCompletion(5, TimeUnit.MINUTES);
}
hold on
i see image name and port written raw in code
wtf
sterile crest
yeah i know
bright lark
oh shit
bright lark
ye it's maybe the next one
timber mirage
i see image name and port written raw in code
Please think about soleving the test problem instead of stopping every 3 minutes to detail that you don't need to stop to
sterile crest
i'm trying to answer all your concerns and questions now @bright lark so that you're not hesistent to approach the problem
bright lark
i'm trying to answer all your concerns and questions now <@357507852560236546> s...
i'm able to do it
but i dnt like it this way
tht's all
and knowing tht my motivation is my only purpose to play with this project
sterile crest
there's 2 types of integration testing you can do
starting from Spring or starting directly with DockerService
timber mirage
but i dnt like it this way
which way ?
sterile crest
DockerService is the easy one to test
bright lark
which way ?
1st, we simplify the core service classes
timber mirage
starting from Spring or starting directly with `DockerService`
Spring, it's the most important one
timber mirage
1st, we simplify the core service classes
We can't simplify without tests
bright lark
sterile crest
yeah we need tests otherwise we won't know if we broke anything during refactoring
we already learned this when i refactored the project
but we wrote tests shortly after learning
sterile crest
so while refactoring is great, we're all for it - we might shoot ourselves in the foot
unless we have tests to protect us from these accidental mistakes
we need a lot in place like tests, diagrams etc. so we know what we're doing
it's a collaborative process, nothing should be on one person
it's just nice that you stepped up to helps us, we appreciate it a lot
bright lark
ok then
sterile crest
but we also have to make sure that we're doing the right things for the project
otherwise, i would have replaced the entire solution with an AWS Lambda ages ago 
bright lark
alright, let imagine now tests are set
and pass without errors
what would be next ? !
sterile crest
take things step by step
sure we can plan the next 10 steps but that's not good use of our time right now
bright lark
why no body else did the tests ?!
sterile crest
because istan was alone and when ala jumped on he was alone
timber mirage
Note that I did tests
for the wrapper that is
because tests for the wrapper were important at the time
but now
what is important
is the tests for the api
I usually put test depending of how much something can break
wrapper that I made was very fragile
so I added tests
the api wasn't fragile back then
because it was much more simple
but it is no longer the case (since istan added the docker service)
so we need tests
@bright lark
@sterile crest
compact holly
because istan was alone and when ala jumped on he was alone
whole project is made by ala, with some help with docker stuff
istannen wrote the previous jshell bot
timber mirage
yea docker is istan, the rest is me
timber mirage
yea it's very small
sterile crest
it's just interacting with loads of moving parts
but the files themselves are not massive either
DockerService is 157 LoC, JShellService 293 LoC
timber mirage
DockerService is 157 LoC, JShellService 293 LoC
And JShellSessionService?
sterile crest
JShellSessionService 133 LoC
the entire API repo is practically less than 1000 LoC
bright lark
it's not about lines
timber mirage
yes it isn't otherwise we wouldn't need tests
sterile crest
but it shows that the project is small and you can easily understand a few lines of code
JShellSessionService
private void initScheduler() {
scheduler = Executors.newSingleThreadScheduledExecutor();
scheduler.scheduleAtFixedRate(() -> {
List<String> toDie = jshellSessions.keySet()
.stream()
.filter(id -> jshellSessions.get(id).shouldDie())
.toList();
LOGGER.info("Scheduler heartbeat, sessions ready to die: {}", toDie);
for (String id : toDie) {
JShellService service = jshellSessions.get(id);
if(service.isMarkedAsDead()) {
try {
jshellSessions.remove(id).close();
} catch (Exception ex) {
LOGGER.error("Unexpected exception for session {}", id, ex);
}
} else {
service.markAsDead();
}
}
}, config.schedulerSessionKillScanRateSeconds(),
config.schedulerSessionKillScanRateSeconds(), TimeUnit.SECONDS);
}
you telling me you really can't understand what's going on here?
private synchronized JShellService createSession(SessionInfo sessionInfo)
throws DockerException {
// Just in case race condition happens just before createSession
if (hasSession(sessionInfo.id())) {
return jshellSessions.get(sessionInfo.id());
}
if (jshellSessions.size() >= config.maxAliveSessions()) {
throw new ResponseStatusException(HttpStatus.TOO_MANY_REQUESTS,
"Too many sessions, try again later :(.");
}
LOGGER.info("Creating session : {}.", sessionInfo);
JShellService service = new JShellService(dockerService, this, sessionInfo.id(),
sessionInfo.sessionTimeout(), sessionInfo.renewable(), sessionInfo.evalTimeout(),
sessionInfo.evalTimeoutValidationLeeway(), sessionInfo.sysOutCharLimit(),
config.dockerMaxRamMegaBytes(), config.dockerCPUsUsage(), config.dockerCPUSetCPUs(),
startupScriptsService.get(sessionInfo.startupScriptId()));
jshellSessions.put(sessionInfo.id(), service);
return service;
}
this is the only other main logic in that class
sure the jumping between classes makes it a bit harder
but look at the code at a bird eye view
and it's all of a sudden not a scary project 😄
and it's why it's a simple codebase to refactor
we just to organise the methods a little
timber mirage
```java
private synchronized JShellService createSession(SessionInfo session...
The thing is that there are like only three things that happen in this method
- case for race condition
- failfast
- new JShellService
Like that's all, this is how simple it is
sterile crest
the naming ala uses can be a bit confusing tho 😄
when he says buzz words like "race conditon"
but insense
it's the same as
String add(String s) {
if(list.contains(s)) {
return list.get(s);
}
return list.add(s);
}```It actually is
The only reason I have this if is in case two threads arrive here at the same time
but yea, it's just a if
sterile crest
yeah but it's just a guard though if we disassociated the "why"
timber mirage
yea
sterile crest
you'd always normally check if it exists before creating
timber mirage
there are 2 guard clause
then logic is just one new + one put to map
bright lark
how to build the img from test class
sterile crest
how to build the img from test class
try some experimentations
part of the fun is problem solving, it's why we're developers 🙂
aim to enjoy yourself
and don't stress without getting involved, outside of us 3, there's the entire server who can help us out if we got stuck
nothing needs to be done in 1 day either so take your time, jshell was rushed to delivery but we can take our time because all things considered, it's not going to harm us - we want to aim for quality after all and be proud of the feature
bright lark
bright lark
try some experimentations
i did
i wonder why we don't have a repo for images ?
it's for free on Docker Hub, and as project is public, we can have a public repo for images
docker images are meant to be stored on a server repo, like we do for code on github
this can significantly simplifies integreation tests
which is my current task
?
bright lark
if we put the wrapper image from a repository, with testconainers we can pull image and run it easily
sterile crest
you can pull it from our self hosted docker repo
the source doesn't matter, it's like doing mavenLocal if this was a gradle dep
bright lark
you can pull it from our self hosted docker repo
i know but in tests, that requires building the img beforeall
and deleting it afterall
by launching gradle commands
and docker commands
sterile crest
yeah but you can do it in gradle itself
test {
dependsOn docker
}
or whatever the name is
so when you run gradle test or whatever command, it'll sort the image out
locally running in intellij, you'll have image pulled already
bright lark
```java
test {
dependsOn docker
}
```
so gradle is able to run a task
sterile crest
ofc?
bright lark
when i run test task ?
sterile crest
yeah
sterile crest
duh? it's one of the primary usecases
what do you think happens when you do gradle build?
it compiles, runs tests, runs spotless etc.
bright lark
it's been while i didn't use gradle
sterile crest
there's also the doFirst doLast blocks you can do too
or if you want to ensure a better ordering mustRunAfter, shouldRunAfter
etc
loads of config available anyway, check out the docs
bright lark
that should help
bright lark
LoggerFactory is not a Logback LoggerContext but Logback is on the classpath. Either remove Logback or the competing implementation (class org.gradle.internal.logging.slf4j.OutputEventListenerBackedLoggerContext loaded from file:/C:/.../.gradle/caches/8.7/generated-gradle-jars/gradle-api-8.7.jar). If you are using WebLogic you will need to add 'org.slf4j' to prefer-application-packages in WEB-INF/weblogic.xml: org.gradle.internal.logging.slf4j.OutputEventListenerBackedLoggerContext
java.lang.IllegalArgumentException: LoggerFactory is not a Logback LoggerContext but Logback is on the classpath. Either remove Logback or the competing implementation (class org.gradle.internal.logging.slf4j.OutputEventListenerBackedLoggerContext loaded from file:/C:/.../.gradle/caches/8.7/generated-gradle-jars/gradle-api-8.7.jar). If you are using WebLogic you will need to add 'org.slf4j' to prefer-application-packages in WEB-INF/weblogic.xml:
org.gradle.internal.logging.slf4j.OutputEventListenerBackedLoggerContext
when i run the test
this is my class
@SpringBootTest
@AutoConfigureMockMvc
public class JShellApiTests {
private static final Logger LOGGER = LoggerFactory.getLogger(Main.class);
@Autowired
private MockMvc mockMvc;
@Test
public void test() {
String codeSnippet = "2 + 2";
try {
mockMvc.perform(MockMvcRequestBuilders.post("/jshell/eval/test")
.contentType(MediaType.APPLICATION_JSON)
.content(codeSnippet))
.andExpect(MockMvcResultMatchers.status().isOk())
.andExpect(MockMvcResultMatchers.jsonPath("$.status").value(SnippetStatus.VALID));
} catch (Exception e) {
LOGGER.error("something went wrong: {}", e.getMessage());
}
}
}
Good stuff, I'm sure you can understand how to fix the error 🙂
bright lark
ive been searching for 1 hour
tried to exclude like this :
test {
dependsOn ':JShellWrapper:jibDockerBuild'
exclude("spring-boot-starter-logging")
exclude("logback-classic")
}
nothing changed
the problem is due to the logger in Main class ig
bright lark
ive got to learn more about Gradle..
timber mirage
ive got to learn more about Gradle..
Any advancement ?
bright lark
Any advancement ?
still learning gradle tricks
yesterday hadn't time for jshell
however i still have weird error
bright lark
```bash
LoggerFactory is not a Logback LoggerContext but Logback is on the class...
^
wby?
timber mirage
```bash
LoggerFactory is not a Logback LoggerContext but Logback is on the class...
@sterile crest can you help him ?
bright lark
also did you change the buil.gradle?
yes ive added a control
to check if img exist, otherwise build it
i have also to add a check when tests end, remove the img
i dunno about jib
if it could remove the img
along the way i learn about gradle custom tasks
timber mirage
yes ive added a control
show it please
bright lark
task buildDockerImage {
group = 'Docker'
description = 'builds jshellwrapper as docker image'
dependsOn jibDockerBuild
}
task removeDockerImage {
group = 'Docker'
description = 'removes jshellwrapper image'
commandLine 'docker', 'rmi', '-f', imageName
}
test {
dependsOn buildDockerImage
finalizedBy removeDockerImage
}
this is not the final config
intellij wants me to use tasks.register()
in addition to some other warnings, it's confusing me
bright lark
here is a final version for tasks
tasks.register('buildDockerImage') {
group = 'Docker'
description = 'builds jshellwrapper as docker image'
dependsOn jibDockerBuild
doFirst{
println('creating jshellwrapper image...')
}
doLast{
println('docker image is ready for use')
}
}
tasks.register('removeDockerImage', Exec) {
group = 'Docker'
description = 'removing jshellwrapper image'
commandLine 'docker', 'rmi', '-f', imageName
doLast{
println('docker image has been removed')
}
}
tasks.named('test') {
dependsOn tasks.named('buildDockerImage')
doFirst {
try {
println 'Running JShellAPI tests...'
} catch (Exception e) {
println 'JShellAPI tests failed'
tasks.named('removeDockerImage').get().execute()
throw e
}
}
doLast {
println 'JShellAPI tests completed.'
}
finalizedBy tasks.named('removeDockerImage')
}
looks nice in execution tbh
time to commit this
sterile crest
<@821143476455342120> can you help him ?
it's a simple error in nature, spring ships with its own logger and there's another version conflicting on the path
it'll be from one of the other dependencies
so what you need to do is analyse the dependency tree and exclude it
bright lark
so what you need to do is analyse the dependency tree and exclude it
the question is
why this doesn't happen when running the app ?
sterile crest
it could also be because gradle's internal logging is conflicting with another lib
because you're not using mockito, junit or any of the testing support during running? ?
they're all libraries being added to the classpath during test
bright lark
i dnt think so
ive tried logging in others apps like this one
never saw this error
sterile crest
lul
bright lark
maybe im wrong
sterile crest
u are
push your code to a branch, i'll look at it later
im free in an hour or so
the main idea is to exclude the conflicting path during the test step
bright lark
alright
i found it
testImplementation ('org.springframework.boot:spring-boot-starter-test') {
configurations {
all {
exclude group: 'org.springframework.boot', module: 'spring-boot-starter-logging'
exclude group: 'ch.qos.logback', module: 'logback-classic'
exclude group: 'org.apache.logging.log4j', module: 'log4j-to-slf4j'
}
}
}
why i can't open a branch on github ?
np im forking the repo
alright, ive pushed changes on a forked repo
what's done in JShellApi subproject :
- custom gradle tasks for tests ✅
- creating a first class for tests ✅
feel free to give me feedback
// ----
what's next :
- set some integration tests for /jshell/eval
- maybe will add some unit tests to protect some methods from breaking
sterile crest
okay but you need to link me to your fork
why did you create a new task called test @bright lark ?
anyways
works for me
once I removed ur garbage
tasks.named('test') {
dependsOn tasks.named('buildDockerImage')
doFirst {
try {
println 'Running JShellAPI tests...'
} catch (Exception e) {
println 'JShellAPI tests failed'
tasks.named('removeDockerImage').get().execute()
throw e
}
}
doLast {
println 'JShellAPI tests completed.'
}
finalizedBy tasks.named('removeDockerImage')
}
delete all of this
and replace it with
test.dependsOn buildDockerImage
testImplementation('org.springframework.boot:spring-boot-starter-test') {
configurations {
all {
exclude group: 'org.springframework.boot', module: 'spring-boot-starter-logging'
exclude group: 'ch.qos.logback', module: 'logback-classic'
exclude group: 'org.apache.logging.log4j', module: 'log4j-to-slf4j'
}
}
}
also all of this, replace with
testImplementation('org.springframework.boot:spring-boot-starter-test') {
configurations {
all {
exclude group: 'ch.qos.logback', module: 'logback-classic'
}
}
}
you can remove testImplementation gradleTestKit() not needed
bright lark
`test.dependsOn buildDockerImage`
i need to remove the img when test is done
sterile crest
why
sterile crest
okay
bright lark
also, it's useful to remove the img if test fails
sterile crest
in that case
I'm certain this isn't needed either
tasks.register('buildDockerImage') {
group = 'Docker'
description = 'builds jshellwrapper as docker image'
dependsOn jibDockerBuild
doFirst{
println('creating docker image...')
}
doLast{
println('docker image is ready for use')
}
}
test.dependsOn jibDockerBuild
sterile crest
import java.time.Instant
plugins {
id 'org.springframework.boot' version '3.2.5'
id 'io.spring.dependency-management' version '1.1.5'
id 'com.google.cloud.tools.jib' version '3.4.2'
id 'com.github.johnrengelman.shadow' version '8.1.1'
}
dependencies {
implementation project(':JShellWrapper')
implementation 'org.springframework.boot:spring-boot-starter-web'
implementation 'com.github.docker-java:docker-java-transport-httpclient5:3.3.6'
implementation 'com.github.docker-java:docker-java-core:3.3.6'
testImplementation('org.springframework.boot:spring-boot-starter-test') {
exclude group: 'ch.qos.logback', module: 'logback-classic'
}
annotationProcessor "org.springframework.boot:spring-boot-configuration-processor"
}
def imageName = 'togetherjava.org:5001/togetherjava/jshellbackend:master' ?: 'latest';
jib {
from.image = 'eclipse-temurin:21'
to {
image = imageName
auth {
username = System.getenv('ORG_REGISTRY_USER') ?: ''
password = System.getenv('ORG_REGISTRY_PASSWORD') ?: ''
}
}
container {
mainClass = 'org.togetherjava.jshellapi.Main'
setCreationTime(Instant.now().toString())
}
}
shadowJar {
archiveBaseName.set('JShellPlaygroundBackend')
archiveClassifier.set('')
archiveVersion.set('')
}
test.dependsOn jibDockerBuild
test.doLast {
project.exec {
commandLine 'docker', 'rmi', '-f', imageName
}
}
there
updated your build.gradle
removed all the extra crap you added for no reason
bright lark
sterile crest
what
bright lark
my impl is better in terms of readability
as it provides grouping, descriptions and logs
those are useful, when performing a build on any env
i tried to put myself in a position of a beginner
i can clean but not like as u did
it's too short
also, it seems that tasks.register() is recommended
sterile crest
recommended by who
bright lark
by gradle docs
sterile crest
my impl is better in terms of readability
how
bright lark
sterile crest
your impl has a million lines of boilerpalte
see you can't read the docs
it's telling you how to create a new task
but the tasks already exist
bright lark
your impl has a million lines of boilerpalte
i can clean
your feedback is great
so we can set something at the middle
sterile crest
sure you can get your PR merged by somebody else then
maybe @timber mirage will do it
sterile crest
what you're doing is:
the eqv of
void print(String s) {
System.out.println("running print");
println(s);
System.out.println("finished print");
}```it makes no sense
cedar fog
That's not what Wazei is saying. Changes should be made for a good reason, and add value. At first glance this PR adds verbosity, with at most an initial beginner ramup easement?
sterile crest
i can get behind creating a gradle task to delete an image but not the other bits
cedar fog
It's not that the sentiment isn't appreciated. But ideally before changing existing functionality, start it from a discussion/ticket point of view, and don't start from a code point of view. A lot of decisions were made for a specific reason.
bright lark
if we a set a task for removing, it would be obvious to set a task to create
and grouping those, is helpful
as for the println() i can reduce or get rid of em
cedar fog
i can get behind creating a gradle task to delete an image but not the other bit...
Is a delete task useful? It's easy to manage yourself.
sterile crest
Is a delete task useful? It's easy to manage yourself.
nope not useful, the only reason i can see for it is that we have a create task so lets have a delete but that isn't normally the task for a build tool anyway (in our context)
cedar fog
nope not useful, the only reason i can see for it is that we have a create task ...
That last bit was indeed my initial thought.
bright lark
if it's not useful, then how should it be ?!
cedar fog
There are a lot of ways to manage the removal of images, but that's usually not managed by your build file.
sterile crest
ideally, if the test needs setup, you'd do it in the @BeforeAll step
bright lark
ideally, if the test needs setup, you'd do it in the `@BeforeAll` step
i was going to do tht
but it was u who suggested to go with gradle build file
bright lark
so i followed that suggestion and found it feasible
also, as the build command is in the build file, it was good to keep those controls in same place
sterile crest
it's not best practise due to inconvincence for maintainability, if we decide gradle isn't for us, switching to maven would be a bigger chore
bright lark
so java testing code will be less verbose and focused on the api
sterile crest
so you'd want to keep a lot of heavy work outside of gradle
but you know, test.dependsOn jibDockerBuild this is a convincence as we already have an existing task
sterile crest
these are gradle specific traps to be careful about and one of the reasons people prefer maven
it's super easy to start doing a lot of logic in your gradle build scripts
timber mirage
maybe <@175710436967055361> will do it
?
sterile crest
just some general knowledge sharing, but you know, people in other places to some crazy things 😄
bright lark
as for tomorrow, we need to push the image on a respository
and use testcontainers. and get rid of those temporary solutions
sterile crest
as for tomorrow, we need to push the image on a respository
it's published to our local repo afaik
sterile crest
local
sterile crest
it's accessible by the VPS only I think? ? togetherjava.org:5001
bright lark
i dnt get this
sterile crest
I might be wrong though, maybe ala or marko can clarify
bright lark
ok
bright lark
```groovy
import java.time.Instant
plugins {
id 'org.springframework.boot' ...
tried this
and it works, but it doesn't remove the img when test fails
and it doesn't remove it when it passes !
id have to create img each time i run tests ?
how this looks fine to u ?
bright lark
hold on
i think i made a mistake
yeah
it deletes the img when test passes, but doesn't remove it when it fails :/
test {
dependsOn jibDockerBuild
doLast {
project.exec {
commandLine 'docker', 'rmi', '-f', imageName
}
}
}
waiting for ur answer @sterile crest
sterile crest
change it to `finalizedBy `
^^
sterile crest
yeah
task removeDockerImage {
doLast {
project.exec {
commandLine 'docker', 'rmi', '-f', imageName
}
}
}
test.finalizedBy removeDockerImage
like that
and you get your removeDockerImage task
bright lark
ahaa
we get back to custom task
without a creation task, it'd be ambiguous to contributors to understand this
sterile crest
ok
bright lark
🤷♂️
im gonna group them under "integration test"
or maybe simply "docker"
here we go
tasks.register('buildDockerImage') {
group = 'docker'
description = 'builds jshellwrapper as docker image'
dependsOn jibDockerBuild
}
tasks.register('removeDockerImage', Exec) {
group = 'docker'
description = 'removes jshellwrapper image'
commandLine 'docker', 'rmi', '-f', imageName
}
test {
dependsOn tasks.named('buildDockerImage')
finalizedBy tasks.named('removeDockerImage')
}
You don't need the tasks.named to ref it
bright lark
def taskBuildDockerImage = tasks.register('buildDockerImage') {
group = 'docker'
description = 'builds jshellwrapper as docker image'
dependsOn jibDockerBuild
}
def taskRemoveDockerImage = tasks.register('removeDockerImage', Exec) {
group = 'docker'
description = 'removes jshellwrapper image'
commandLine 'docker', 'rmi', '-f', imageName
}
test {
dependsOn taskBuildDockerImage
finalizedBy taskRemoveDockerImage
}
Yeee
bright lark
sterile crest
Kinda iffy about the first one because it's a task to rename and existing one but fineee
Btw
Use the short form like I showed in my task
Instead of tasks.registrr just task name { }
bright lark
Instead of tasks.registrr just `task name { }`
why not ?
sterile crest
Well it doesn't matter because all the docs are there for the registered one
bright lark
Well it doesn't matter because all the docs are there for the registered one
read this
sterile crest
Why
sterile crest
I don't see the recommendation
But do what you want
I don't wanna spend time going back and fourth on this
You're unblocked anyway
bright lark
i have a weird error
2024-08-10T16:02:23.809+01:00 INFO 21152 --- [pool-2-thread-1] o.t.j.service.JShellSessionService : Scheduler heartbeat: sessions ready to die: []
2024-08-10T16:02:23.810+01:00 WARN 21152 --- [o-auto-1-exec-1] o.t.jshellapi.service.JShellService : Unexpected error during creation.
com.github.dockerjava.api.exception.InternalServerErrorException: Status 500: {"message":"Head \"https://togetherjava.org:5001/v2/togetherjava/jshellwrapper/manifests/master\": no basic auth credentials"}
i need credentials to spawn the container ?
i dunno why this didn't happen when i run the app
i think the spawnContainer() in dockerservice is wrong
ah i get it
the image name is different
i made a mistake in gradle build file, the command should beJShellWrapper:jibDockerBuild
timber mirage
yes
jib -> create the image in the server, needs cradentials
jibDockerBuild -> create the image in local
@bright lark
btw why do you need to delete the image ?
bright lark
btw why do you need to delete the image ?
locally
when test is done
if i keep it. When i rerun test, new img is created
im gonna have to inject jshelwrapper image name from root gradle file
alright, ive finished on test so far
it took time honestly but it works
and i think it would be a good idea to review my changes in a first PR
so next tests are rebased on it
👉 im gonna send pr
timber mirage
and i think it would be a good idea to review my changes in a first PR
do you at least one working test ?
timber mirage
if i keep it. When i rerun test, new img is created
so it's pointless to delete it ?
bright lark
do you at least one working test ?
yes
bright lark
so it's pointless to delete it ?
wdym
timber mirage
also can you chose different kind of config in the tests ?
bright lark
why are you deleting the image ?
cuz tht's the scenario
timber mirage
?
bright lark
- build img
- start api server
- perform api call
- run container from img
- gets code result
- shutdown container
- delete image
timber mirage
but why do you need to delete the image ?
timber mirage
and ?
bright lark
also i told u if keep it
everytime i run tests, new image is added
this is cumbersome
timber mirage
why ?
bright lark
i only need 1 img
timber mirage
why ?
bright lark
it's close to real scenario
bright lark
alright, consider performin the tests on ci/CD
it would be better to delete it once tests are done
bright lark
wdym
timber mirage
so about the tests
timber mirage
- testing eval results
- testing single eval
- testing eval edge case behavior (while(true), crash, etc)
- testing the sessions (sending several code one after each other and seeing it works)
- testing all kinds of timeouts
- testing snippets
- testing shutdown (especially edge cases)
and probably more
bright lark
i know
but id submit my pr now
to review the approach
on which we can rebase for other tests
we can keep the issue open and have multiple prs
timber mirage
yes
bright lark
timber mirage
@compact holly @sterile crest can you review this please ? ↑ Since you know way better than me how gradle works
@bright lark what is this https://github.com/Together-Java/JShellPlaygroundBackend/blob/3502cb76912531531a5259b047e1eb5cb6f8089c/JShellAPI/src/main/java/org/togetherjava/jshellapi/rest/ApiEndpoints.java ?
bright lark
<@357507852560236546> what is this https://github.com/Together-Java/JShellPlaygr...
constants class
for the endpoints
timber mirage
constants class
That you don't use ?
bright lark
That you don't use ?
i used in tests
timber mirage
i used in tests
Then why is it here ?
bright lark
hmm
it should be under /test
ye mb
well no
i forgot to synchronize it with the paths in controller
the idea is to keep paths same between controllers and test classes
sterile crest
@bright lark how does additional-spring-configuration-metadata.json work?
bright lark
<@357507852560236546> how does `additional-spring-configuration-metadata.json` w...
it helps to know more about the custom properties
sterile crest
how?
sterile crest
ah i see
bright lark
it shows an error
sterile crest
it's so the IDE can enforce a type
what a trash way to do, spring sucks 😦
it's trash because it's just used for IDE linting, it doesn't need to be included in the final built jar
stray wrenBOT
sterile crest
JShell has been down for awhile now
What's up?
Just gonna leave it in a broken state or what?
bright lark
hello
@sterile crest as for diagrams, i ve first reviewed alathreon's PR in which he added an overview diagram
i can add some diagrams, i already did some
but i dunno if that would be acceptable
sterile crest
make more
bright lark
i did, i just kept them on my laptop
bright lark
make more
but as i recall, marko said that doing such , will make the project look like a real product dedicated for commercial
and that's not the objective
while my intention is to provide clear overview to us and newcomers
also, there is something else before the diagrams, writing some documentation about the usecases and actors
following Alistar cockburn model
timber mirage
.
@sterile crest I am talking about this pr https://github.com/Together-Java/JShellPlaygroundBackend/pull/49
It's been a month that it is waiting for a review
timber mirage
@sterile crest I merged it and created and merge a pr and master
And @compact holly just to tell you that the jshell backend just got updated
timber mirage
done ! https://github.com/Together-Java/JShellPlaygroundBackend/pull/50
I also would like that @sterile crest and @compact holly review this PR, since @bright lark is editing a lot of config, and you are much better than me when it comes to gradle
sterile crest
Looks like it's still having issues
The warning logs are going crazy with the NPE that's being fired
timber mirage
The warning logs are going crazy with the NPE that's being fired
what are you talking about ?
org.togetherjava.tjbot.features.help.MarkHelpThreadCloseInDBRoutine
WARN
unable to mark thread as close with id :1269585872718790667
java.lang.NullPointerException: Cannot invoke "net.dv8tion.jda.api.entities.channel.concrete.ThreadChannel.getIdLong()" because "threadChannel" is null
at org.togetherjava.tjbot.features.help.HelpThreadLifecycleListener.handleArchiveStatus(HelpThreadLifecycleListener.java:86)
at org.togetherjava.tjbot.features.help.MarkHelpThreadCloseInDBRoutine.lambda$updateTicketStatus$1(MarkHelpThreadCloseInDBRoutine.java:66)
at java.base/java.lang.Iterable.forEach(Iterable.java:75)
at org.togetherjava.tjbot.features.help.MarkHelpThreadCloseInDBRoutine.updateTicketStatus(MarkHelpThreadCloseInDBRoutine.java:63)
at org.togetherjava.tjbot.features.help.MarkHelpThreadCloseInDBRoutine.runRoutine(MarkHelpThreadCloseInDBRoutine.java:49)
at org.togetherjava.tjbot.features.system.BotCore.lambda$scheduleRoutines$7(BotCore.java:199)
at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572)
at java.base/jav...
this ?
this has nothing to do with jshell
sterile crest
ah yeah, I was looking at it then stopped looking at the class it was erroring in
log blindness 😦
bright lark
Whatsp
timber mirage
🤷
compact holly
no issues with jshell so far?
timber mirage
I also would like that <@821143476455342120> and <@238042761490595843> review th...
@compact holly -_-
compact holly
i mean bugs, is it stable?
timber mirage
i mean bugs, is it stable?
No issue so far, but please review firas PR
sterile crest
No issue so far, but please review firas PR
you review it, it's your project
if you don't understand the tech then why did you choose it 
timber mirage
you review it, it's your project
It's not mine, it's ours
timber mirage
if you don't understand the tech then why did you choose it <:angerysad:51094906...
I didn't chose gradle
If it was maven, yes I could review it
but i don't know gradle
sterile crest
learn it!
timber mirage
and I am not the one how chose it
sterile crest
the time firas's PR has been around, you could have mastered gradle
timber mirage
learn it!
I need someone who is experienced in it, it doesn't matter if I learn it, we need someone who is experienced in gradle
compact holly
nah, it's fine, it's not on ala to do everything
it's our project, and everyone should contribute what they can
now, it's true that it's only us here, and nobody knows or really cares about this project
but that's the story for another time
we do have to think about maintainability and long-term sustainability
sterile crest
I need someone who is experienced in it, it doesn't matter if I learn it, we nee...
the thing is, you can use your initative. Does everything still work? Do the changes look sensible? Check there isn't a "rm -r System32" in there and if it looks good, give it a tick
I didn't even review your PRs i just ticked them for example 
compact holly
it's also fine to wait for domain experts to review, or codeowners
if you are not comfortable reviewing the code
it's actually preferred, since we care about quality more than just pushing changes (that are not rm system32 or keyloggers)
timber mirage
the thing is, you can use your initative. Does everything still work? Do the cha...
...seriously please
I am expecting that any pr are at elast correctly reviewed
compact holly
and we care about stability more than the new features
timber mirage
so don't lie in your reviews
sterile crest
LGTM
compact holly
it would be tragic if adding tests (attempt to make software more stable), actualy takes down our production service
sterile crest
...seriously please
I am expecting that any pr are at elast correctly reviewed
then i'm not the best person to ask in the current time
i just came from home from the airport
timber mirage
@sterile crest @compact holly if nobody review it or tell that they will review it before saturday, i'll merge it
compact holly
that's fair at the current project state, but not the best approach overall
what I really dislike is jshell being broken for a month, that shouldn't ever happen on our production services
at if it happens, that feature should be disabled, with a friendly explanation
so at least there should be pipeline to make sure everything builds, and few smoke screen/integration tests that make sure that basic functionality is there (i'm aware pr is about that)
and you want actual reviews not only to ensure stability and maintainability (which is crucial for the project)
but also to engege community with the project, and share knowledge
if project is a black box made by a single member, not documented enough, doesn't have good guides and easy to do issues.. nobody will join
because it's a daunting task, that would require a week for an experienced dev to delve deep into it and reproduce and fix a bug
timber mirage
if project is a black box made by a single member, not documented enough, doesn'...
Yes but that also takes a lot of time
compact holly
yup, time and effort
like everything in life.. 
but anyway, whatever we do, it should at least be stable enough
broken features should be disabled until fixed
or just revert them to the previous working state
timber mirage
but anyway, whatever we do, it should at least be stable enough
It currently works great
compact holly
yup, everything else is seconodary
so just merging random commits you are unsure about, that are not tested or at least built locally
is not the best approach for keeping it working great
timber mirage
so just merging random commits you are unsure about, that are not tested or at l...
Yea but here I have no choice, will I will test it locally, but that's not what is needed
compact holly
yeah, but it's the most important step
if you are sure it works, sure merge it, fuck it
rest of the codebase is not reviewed either
compact holly
I wouldnt say so, 95% is not
compact holly
I didn't even review your PRs i just ticked them for example <:fat_laugh:5546441...
and wazei admitted to fake reviewing, and sabotaging project from the inside 
compact holly
yeah, it's creative
bright lark
oh hello Oh boy
timber mirage
oh hello Oh boy
so if nobody else review your pr, I'll merge it this friday
se we will be able to advance
bright lark
huh U can't see it?
timber mirage
?
bright lark
If u can see it, then u can review it
bright lark
Ah! Then ask me in pr to explain it somewhere
Probably in readme
It's good tht we got this reaction now in order to do what should be done
As for what marko said, id agree to write some docs ideally with some diagrams
Ala did some progress in this and i reviewed his pr some weeks ago
Also, i did some docs to usecases with other sort of diagrams but kept it locally as i didn't want to share before we merge ala's pr
timber mirage
Also, i did some docs to usecases with other sort of diagrams but kept it locall...
can you focus on tests instead please ?
bright lark
I think i did
And we agreed tht my pr about testing is a first to refine and agree on a strategy for the rest
bright lark
I dunno, why would i write N tests if perhaps they'll be rejected?
timber mirage
wdym
bright lark
so the next things is to do actual tests
I did one test rather than many to see if the approach i followed is accepted
If it's not ok or lacks something
Then same reviews apply to next tests
Whether it's me or others
timber mirage
I did one test rather than many to see if the approach i followed is accepted
well, after reading your pr, the problem seems to be that the tests don't scale well, you are using constants everywhere for the variables to send and the tests has a lot of code just to test a 2+2
you should refactor it so you only need minimal lines of code
also, I would want a second test where you a second request to see if your way of doing the tests can handle state
i think those 2 problems are the biggest problems that i see in your pr
bright lark
Good notes! Please write in pr
and Ill handle godwilling
You're right about the constants some should be inside the test method. As for the endpoint constants i see them useful to maintain sync between controllers and test classes
I admit tht i was a bit hasty before my latest commit
But i can fix in pr review process (which is made for tht)
timber mirage
Good notes! Please write in pr
I did
sterile crest
he is sabotaging so he can use his aws solution 👀
No
sterile crest
and wazei admitted to fake reviewing, and sabotaging project from the inside <:L...
I made it clear when I did the review, the PRs were stab at the dark fixes to see if the problem goes away
At least the main big problem has been resolved which is containers are being destroyed
Any other PR that pops up, idc about, I have no need to review. They are not for me and I'm only interested in bug fixes
timber mirage
@compact holly here you go first good first issue https://github.com/Together-Java/JShellPlaygroundBackend/issues/53
compact holly
<@238042761490595843> here you go first good first issue https://github.com/Toge...
very cute 
sterile crest
@timber mirage I swear I see "session running" but for the life of me can't find who invoked the command 
I just wanted to see who was using it and where and what
Consider creating an issue to show these metrics?
timber mirage
Consider creating an issue to show these metrics?
Just look at the issue just above your message 🙂
sterile crest
I didn't understand that issue
timber mirage
I was like what
what is off ?
compact holly
nope
sterile crest
Can I do it for jshell
timber mirage
Do we use graphana at all?
What's that ?
compact holly
what do you actually need
I don't want to introduce huge systems that someone has to understand, maintain.. and all that
sterile crest
What's that ?
sterile crest
I don't want to introduce huge systems that someone has to understand, maintain....
It's small effort
compact holly
infra stuff should be as simple as possible
depends what you do with it, but it's pretty big boy tool
timber mirage
https://grafana.com/
How will it work ? Is it a TJ feature or JShell backend feature ?
compact holly
when you want to integrate all these tools on a fancy graph, centralized logging and monitoring
you add prometheus
sterile crest
Think of a class called metric that contains a single Map<String, String> and the rest is abstracted away not ever needing to be touched
compact holly
ye, if you can add a monitoring and metrics system on the vps
timber mirage
Note that there is already a pr about this
sterile crest
There isn't
sterile crest
That's some noob version
sterile crest
It can be minimal, i.e. just a Map<String, String>
Then a single method to pop it into graphana
Or whatever we want
timber mirage
Currently, with this pr, it returns some info
but there are two improvments to make :
- also show docker stats about any container starting with session_
compact holly
yeah, but graphs are sexy
timber mirage
- also show deleted sessions
compact holly
data is just.. some numbers
timber mirage
but that's all
sterile crest
I have 2 things on my TODO list for TJ
AI based top helper check
And graphana for jshell
But once graphana is done, it's a plug and play for every other service
compact holly
i have no idea how many people are using it
or any metrics about it
well, to be honest, I have no metrics at all on this server
timber mirage
And graphana for jshell
wait, do you want to add graphana on tj bot project, or jshell backend project ?
sterile crest
Idk who uses it and where, I'm under the impression there's fake logging
compact holly
on tjbot*
sterile crest
wait, do you want to add graphana on tj bot project, or jshell backend project ?
Everything tbh
Even the website
timber mirage
but it's true that we need stats on tj bot
I was asking this since a long time ago
timber mirage
I am pretty sure I made several #server-suggestions about it, and I am not the only one
And I am pretty sure people already did PR for it
compact holly
it would be really nice to actually know what the fuck is happening, how are users using our software
and so on
but we all agree on that
sterile crest
It's a solved problem though, we have all the knowledge to do it
timber mirage
so wazei
sterile crest
Just about waiting till somebody can be bothered
sterile crest
You're implementing it wrong
timber mirage
with the pr I linked earlier, I will improve it a bit
sterile crest
Your pr isn't compatible
timber mirage
so you can just call it to feed graphana
timber mirage
Your pr isn't compatible
why ?
sterile crest
Problem we have is that it's 2 repos and we don't have a common library
So c&p but whatever
compact holly
we can have a common library
sterile crest
we can have a common library
Yeah I'm scared of future problems with that approach
timber mirage
My PR is about monitoring jshell state
What you want is to collect stats of anything so they can be checke later
Right @sterile crest ?
compact holly
me as well, has to be maintained as such