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

feat: reimplement rserve route #813

Merged
merged 29 commits into from
Feb 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
434a48b
feat: reimplement rserve route
marikaris Oct 25, 2024
47a8a1c
Merge branch 'master' into feat/#809-vanilla-rserve
marikaris Oct 31, 2024
97fd12a
chore: add forgotten file
marikaris Oct 31, 2024
214609e
test: add in rserve tests again
marikaris Nov 1, 2024
e3b8ced
feat: reimplement rserve (restore the lost files)
marikaris Nov 1, 2024
1dae824
test: add tests for install image
marikaris Nov 1, 2024
5d620d2
test: write test for toEnvironmentConfigProps
marikaris Nov 1, 2024
46f63be
increase coverage
marikaris Nov 1, 2024
622705b
increase cov
marikaris Nov 1, 2024
fa0e702
test(EnvironmentConfigProps): add tests to increase coverage
marikaris Nov 1, 2024
70b82b4
refactor: extract function to allow for testing
marikaris Nov 11, 2024
df701b0
test(RserverConnectionFactory): add tests for method
marikaris Nov 11, 2024
0236373
refactor: extract function to allow for testing
marikaris Nov 11, 2024
1c0a9b5
test(RserverConnectionFactoryTest): add tests to increase coverage
marikaris Nov 11, 2024
f79b66b
refactor: remove unused code
marikaris Nov 11, 2024
4173dc7
refactor: rename weird possessed warning message into something sensible
marikaris Nov 11, 2024
729ac07
test(RserveConnection): add tests
marikaris Nov 12, 2024
c5db693
test(RserveResult): add tests
marikaris Nov 12, 2024
c39d09c
test(RserveConnectionFactoryTest): add test
marikaris Nov 12, 2024
5de34b8
refactor(ProfileConfig): remove confusing try catch
marikaris Nov 14, 2024
ed3fb1d
chore: make releasetest work with comma seperated list of profiles
marikaris Jan 24, 2025
4c00bda
Merge branch 'master' into feat/#809-vanilla-rserve
marikaris Jan 24, 2025
865834a
Merge branch 'master' into feat/#809-vanilla-rserve
marikaris Jan 28, 2025
9c7a977
ci: run release test on vanilla rserve (xenon) and rock (donkey)
marikaris Jan 28, 2025
3046e76
ci: update docker compose
marikaris Jan 28, 2025
c61d61c
ci: change order of profiles
marikaris Jan 28, 2025
f08c3ff
ci: see if vanilla rserve on other port does the trick
marikaris Jan 28, 2025
c6ae411
ci: remove unavailable profile configuration to fix releasetest
marikaris Feb 18, 2025
7661e4d
Merge branch 'master' into feat/#809-vanilla-rserve
marikaris Feb 18, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public EnvironmentConfigProps toEnvironmentConfigProps() {
props.setName(getName());
props.setHost(getHost());
props.setPort(getPort());
props.setImage(getImage());
return props;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public Map<String, ContainerInfo> getAllProfileStatuses() {
*/
String asContainerName(String profileName) {
if (!inContainer) {
LOG.warn("NO ".repeat(100) + " " + profileName);
LOG.warn("Profile not running in docker container: " + profileName);
return profileName;
}

Expand All @@ -104,7 +104,7 @@ String asContainerName(String profileName) {
return profileName;
}

LOG.warn("YES ".repeat(100) + " " + profileName);
LOG.warn("Profile running in docker container: " + profileName);
return containerPrefix + profileName + "-1";
}

Expand Down Expand Up @@ -148,12 +148,13 @@ public void startProfile(String profileName) {
startContainer(containerName);
}

private void installImage(ProfileConfig profileConfig) {
void installImage(ProfileConfig profileConfig) {
if (profileConfig.getImage() == null) {
throw new MissingImageException(profileConfig.getImage());
}

int imageExposed = 8085;
// if rock is in the image name, it's rock
int imageExposed = profileConfig.getImage().contains("rock") ? 8085 : 6311;
ExposedPort exposed = ExposedPort.tcp(imageExposed);
Ports portBindings = new Ports();
portBindings.bind(exposed, Ports.Binding.bindPort(profileConfig.getPort()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@
import static org.molgenis.armadillo.controller.ArmadilloUtils.createRawResponse;

import java.nio.charset.Charset;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.molgenis.r.RServerResult;
import org.molgenis.r.exceptions.RExecutionException;
import org.molgenis.r.rock.RockResult;
import org.molgenis.r.rserve.RserveResult;
import org.rosuda.REngine.REXPRaw;

@ExtendWith(MockitoExtension.class)
Expand All @@ -27,18 +26,11 @@ void testSerializeCommand() {
assertEquals("try(base::serialize({meanDS(D$age}, NULL))", serializedCommand);
}

@Disabled
@Test
void testCreateRawResponse() {
byte[] bytes = {0x01, 0x02, 0x03};
REXPRaw rexpDouble = new REXPRaw(bytes);
// rexpDouble renders to String "org.rosuda.REngine.REXPRaw@3a3f96ab[3]"
byte[] actual = createRawResponse(new RockResult(rexpDouble));
StringBuilder sb = new StringBuilder();
for (byte b : actual) {
sb.append((char) b);
}
String str = sb.toString();
byte[] actual = createRawResponse(new RserveResult(rexpDouble));
assertArrayEquals(bytes, actual);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.security.Principal;
import java.util.*;
import java.util.concurrent.CompletableFuture;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
Expand All @@ -45,6 +44,7 @@
import org.molgenis.armadillo.storage.ArmadilloStorageService;
import org.molgenis.r.model.RPackage;
import org.molgenis.r.rock.RockResult;
import org.molgenis.r.rserve.RserveResult;
import org.obiba.datashield.core.DSEnvironment;
import org.obiba.datashield.core.DSMethod;
import org.obiba.datashield.core.impl.DefaultDSMethod;
Expand Down Expand Up @@ -288,7 +288,6 @@ void getAggregateMethods() throws Exception {
Map.of("sessionId", sessionId, "roles", List.of("ROLE_USER"))));
}

@Disabled
@Test
@WithMockUser
void testGetLastResultNoResult() throws Exception {
Expand All @@ -297,13 +296,12 @@ void testGetLastResultNoResult() throws Exception {
mockMvc.perform(asyncDispatch(result)).andExpect(status().isNotFound());
}

@Disabled
@Test
@WithMockUser
void testGetLastResult() throws Exception {
byte[] bytes = {0x0, 0x1, 0x2};
when(commands.getLastExecution())
.thenReturn(Optional.of(completedFuture(new RockResult(new REXPRaw(bytes)))));
.thenReturn(Optional.of(completedFuture(new RserveResult(new REXPRaw(bytes)))));

MvcResult result =
mockMvc.perform(get("/lastresult").accept(APPLICATION_OCTET_STREAM)).andReturn();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package org.molgenis.armadillo.metadata;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.molgenis.armadillo.metadata.ProfileConfig.create;

import java.util.HashMap;
import java.util.HashSet;
import org.junit.jupiter.api.Test;
import org.molgenis.r.config.EnvironmentConfigProps;

public class ProfileConfigTest {

@Test
public void testToEnvironmentConfigProps() {
String name = "myName";
String img = "myImage";
String host = "localhost";
int port = 6311;
ProfileConfig config =
create(name, img, host, port, new HashSet<>(), new HashSet<>(), new HashMap<>());
EnvironmentConfigProps actual = config.toEnvironmentConfigProps();
assertEquals(img, actual.getImage());
}

@Test
public void testToEnvironmentConfigPropsDoesNotThrowErrorWhenImageNull() {
ProfileConfig config =
create(
"myName", null, "localhost", 6311, new HashSet<>(), new HashSet<>(), new HashMap<>());
assertDoesNotThrow(config::toEnvironmentConfigProps);
}

@Test
public void testCreateEmptyHost() {
ProfileConfig config =
create("myName", null, null, 6311, new HashSet<>(), new HashSet<>(), new HashMap<>());
assertEquals("localhost", config.getHost());
}

@Test
public void testCreateEmptyOptions() {
ProfileConfig config =
create("myName", null, null, 6311, new HashSet<>(), new HashSet<>(), null);
assertEquals("java.util.ImmutableCollections$MapN", config.getOptions().getClass().getName());
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
package org.molgenis.armadillo.config;
package org.molgenis.armadillo.profile;

import static java.util.Collections.emptyMap;
import static java.util.Collections.emptySet;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand All @@ -29,8 +28,6 @@
import org.molgenis.armadillo.metadata.ProfileConfig;
import org.molgenis.armadillo.metadata.ProfileService;
import org.molgenis.armadillo.metadata.ProfileStatus;
import org.molgenis.armadillo.profile.ContainerInfo;
import org.molgenis.armadillo.profile.DockerService;

@ExtendWith(MockitoExtension.class)
class DockerServiceTest {
Expand Down Expand Up @@ -125,6 +122,23 @@ void testStartProfileNoImage() {
assertThrows(MissingImageException.class, () -> dockerService.startProfile("default"));
}

@Test
void testInstallImageNull() {
ProfileConfig profileConfig = mock(ProfileConfig.class);
when(profileConfig.getImage()).thenReturn(null);
assertThrows(MissingImageException.class, () -> dockerService.installImage(profileConfig));
}

@Test
void testInstallImage() {
ProfileConfig profileConfig = mock(ProfileConfig.class);
String image = "datashield/rock-something-something:latest";
when(profileConfig.getImage()).thenReturn(image);
when(profileConfig.getPort()).thenReturn(6311);
assertDoesNotThrow(() -> dockerService.installImage(profileConfig));
verify(dockerClient).createContainerCmd(image);
}

@SuppressWarnings("ConstantConditions")
@Test
void testStartProfile() {
Expand Down
12 changes: 8 additions & 4 deletions docker/ci/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,17 @@ armadillo:
datashield:
# the seed can only be 9 digits
seed: 342325352
- name: rock
image: datashield/rock-base:latest
host: rock
port: 8085
- name: xenon-vanilla
image: datashield/armadillo-rserver_caravan-xenon:latest
host: xenon-vanilla
port: 6012
package-whitelist:
- dsBase
- resourcer
- dsMediation
- dsMTLBase
- dsSurvival
- dsOmics
function-blacklist: [ ]
options:
datashield:
Expand Down
2 changes: 0 additions & 2 deletions docker/ci/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,3 @@ services:
xenon:
hostname: xenon
image: datashield/rock-dolomite-xenon:latest
environment:
DEBUG: "TRUE"
57 changes: 38 additions & 19 deletions r/src/main/java/org/molgenis/r/RServerConnectionFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

import java.io.IOException;
import java.net.*;
import java.util.Objects;
import org.molgenis.r.config.EnvironmentConfigProps;
import org.molgenis.r.rock.RockConnectionFactory;
import org.molgenis.r.rserve.RserveConnectionFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -46,24 +48,45 @@ RockStatusCode doHead(String uri) {
}
}

String getMessageFromStatus(RockStatusCode statusCode, String url) {
if (statusCode == RockStatusCode.SERVER_DOWN) {
return ("Container for '" + url + "' is down");
} else if (statusCode == RockStatusCode.SERVER_NOT_READY) {
return ("Container for '" + url + "' is not ready");
} else if (statusCode == RockStatusCode.OK) {
return ("Container for '" + url + "' is running");
} else if (statusCode == RockStatusCode.UNEXPECTED_RESPONSE_CODE) {
return ("Unexpected response code on " + url);
} else if (statusCode == RockStatusCode.UNEXPECTED_RESPONSE) {
return ("Unexpected response on " + url);
} else if (statusCode == RockStatusCode.UNEXPECTED_URL) {
return ("MalformedURLException on " + url);
} else {
return "";
}
}

Boolean isWarningStatus(RockStatusCode rockStatusCode) {
return rockStatusCode == RockStatusCode.SERVER_DOWN
|| rockStatusCode == RockStatusCode.SERVER_NOT_READY
|| rockStatusCode == RockStatusCode.UNEXPECTED_URL;
}

@Override
public RServerConnection tryCreateConnection() {
String url = "http://" + environment.getHost() + ":" + environment.getPort();
RockStatusCode rockStatus = doHead(url);
if (rockStatus == RockStatusCode.SERVER_DOWN) {
logger.warn("Container for '" + url + "' is down");
} else if (rockStatus == RockStatusCode.SERVER_NOT_READY) {
logger.warn("Container for '" + url + "' is not ready");
} else if (rockStatus == RockStatusCode.OK) {
logger.info("Container for '" + url + "' is running");
} else if (rockStatus == RockStatusCode.UNEXPECTED_RESPONSE_CODE) {
logger.info("Unexpected response code on " + url);
} else if (rockStatus == RockStatusCode.UNEXPECTED_RESPONSE) {
logger.info("Unexpected response on " + url);
} else if (rockStatus == RockStatusCode.UNEXPECTED_URL) {
logger.warn("MalformedURLException on " + url);
if (environment.getImage().contains("rock")) {
String url = "http://" + environment.getHost() + ":" + environment.getPort();
RockStatusCode rockStatus = doHead(url);
String statusMessage = getMessageFromStatus(rockStatus, url);
if (isWarningStatus(rockStatus)) {
logger.warn(statusMessage);
} else if (!Objects.equals(statusMessage, "")) {
logger.info(statusMessage);
}
return new RockConnectionFactory(environment).tryCreateConnection();
} else {
return new RserveConnectionFactory(environment).tryCreateConnection();
}
return new RockConnectionFactory(environment).tryCreateConnection();
}
}

Expand All @@ -80,8 +103,4 @@ enum RockStatusCode {
RockStatusCode(int code) {
this.code = code;
}

public int getCode() {
return this.code;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
public class EnvironmentConfigProps {
@NotEmpty private String name;
@NotEmpty private String host = "localhost";
private String image = "";
@Positive private int port = 6311;

public String getHost() {
Expand All @@ -31,4 +32,12 @@ public String getName() {
public void setName(String name) {
this.name = name;
}

public String getImage() {
return image;
}

public void setImage(String image) {
this.image = image;
}
}
Loading
Loading