Skip to content

Commit

Permalink
[HOPSWORKS-2057] Fix installing multiple python libs, fix removing co…
Browse files Browse the repository at this point in the history
…nda environments (#686)
  • Loading branch information
tkakantousis authored Oct 5, 2020
1 parent 274fc24 commit 761e7de
Show file tree
Hide file tree
Showing 10 changed files with 157 additions and 56 deletions.
10 changes: 8 additions & 2 deletions hopsworks-IT/src/test/ruby/spec/conda_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -279,12 +279,17 @@
it 'install libraries' do
@project = create_env_and_update_project(@project, python_version)
install_library(@project[:id], python_version, 'imageio', 'conda', '2.2.0', conda_channel)
install_library(@project[:id], python_version, 'tflearn', 'pip', '0.3.2', conda_channel)
expect_status(201)

get_library_commands(@project[:id], python_version, 'imageio')
expect_status(200)
expect(json_body[:count]).to be == 1

get_library_commands(@project[:id], python_version, 'tflearn')
expect_status(200)
expect(json_body[:count]).to be == 1

wait_for do
CondaCommands.find_by(project_id: @project[:id]).nil?
end
Expand All @@ -293,8 +298,9 @@
expect_status(200)
expect(json_body[:count]).to be == 0

install_library(@project[:id], python_version, 'tflearn', 'pip', '0.3.2', conda_channel)
expect_status(201)
get_library_commands(@project[:id], python_version, 'tflearn')
expect_status(200)
expect(json_body[:count]).to be == 0

wait_for do
CondaCommands.find_by(project_id: @project[:id]).nil?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,16 @@ public void deleteCommandsForEnvironment(Project proj) {
}
}

public void deleteCommandsForProject(Project proj) {
List<CondaCommands> commands = getCommandsForProject(proj);
for (CondaCommands cc : commands) {
// delete the conda library command if it has the same name as the input library name
if (cc.getOp().equals(CondaOp.CREATE) || cc.getOp().equals(CondaOp.EXPORT)) {
em.remove(cc);
}
}
}

public void removeCondaCommand(int commandId) {
CondaCommands cc = findCondaCommand(commandId);
if (cc != null) {
Expand All @@ -130,6 +140,16 @@ public List<CondaCommands> findByStatusAndCondaOp(CondaStatus status, CondaOp op
query.setParameter("op", op);
return query.getResultList();
}

public List<CondaCommands> findByStatusAndCondaOpAndProject(List<CondaStatus> statuses, CondaOp op, Project project) {
TypedQuery<CondaCommands> query =
em.createNamedQuery("CondaCommands.findByStatusListAndCondaOpAndProject", CondaCommands.class);
query.setParameter("statuses", statuses);
query.setParameter("op", op);
query.setParameter("project", project);

return query.getResultList();
}

public CondaCommands findCondaCommand(int commandId) {
return em.find(CondaCommands.class, commandId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,12 @@ public PythonDep condaOp(CondaOp op, Users user, CondaInstallType installType, P
}

public void updateCondaCommandStatus(int commandId, CondaStatus condaStatus, String arg,
CondaOp opType) throws ServiceException {
CondaOp opType) throws ServiceException, ProjectException {
updateCondaCommandStatus(commandId, condaStatus, arg, opType, null);
}

public void updateCondaCommandStatus(int commandId, CondaStatus condaStatus, String arg, CondaOp opType,
String errorMessage) throws ServiceException {
String errorMessage) throws ServiceException, ProjectException {
CondaCommands cc = condaCommandFacade.findCondaCommand(commandId);
if (cc != null) {
if (condaStatus == CondaStatus.SUCCESS) {
Expand All @@ -142,18 +142,22 @@ public void updateCondaCommandStatus(int commandId, CondaStatus condaStatus, Str
// the CondaEnv operation is finished implicitly (no condaOperations are
// returned => CondaEnv operation is finished).
if (!CondaOp.isEnvOp(opType)) {
Project project = projectFacade.findById(cc.getProjectId().getId()).orElseThrow(() -> new ProjectException(
RESTCodes.ProjectErrorCode.PROJECT_NOT_FOUND, Level.FINE, "projectId: " + cc.getProjectId().getId()));
PythonDep dep = libraryFacade.getOrCreateDep(libraryFacade.getRepo(cc.getChannelUrl(), false),
cc.getInstallType(), cc.getLib(), cc.getVersion(), true, false);
Collection<PythonDep> deps = cc.getProjectId().getPythonDepCollection();
Collection<PythonDep> deps = project.getPythonDepCollection();

if (opType.equals(CondaOp.INSTALL)) {
deps.remove(dep);
deps.add(dep);
} else if (opType.equals(CondaOp.UNINSTALL)) {
deps.remove(dep);
}
cc.getProjectId().setPythonDepCollection(deps);
projectFacade.update(cc.getProjectId());

project.setPythonDepCollection(deps);
projectFacade.update(project);
projectFacade.flushEm();
}
} else if(condaStatus == CondaStatus.FAILED) {
cc.setStatus(condaStatus);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@
*/
package io.hops.hopsworks.common.python.environment;

import io.hops.hopsworks.exceptions.ProjectException;
import io.hops.hopsworks.exceptions.ServiceException;

import java.io.IOException;

public interface DockerRegistryMngr {

void gc() throws IOException, ServiceException;
void gc() throws IOException, ServiceException, ProjectException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import io.hops.hopsworks.common.util.ProcessResult;
import io.hops.hopsworks.common.util.ProjectUtils;
import io.hops.hopsworks.common.util.Settings;
import io.hops.hopsworks.exceptions.ProjectException;
import io.hops.hopsworks.exceptions.ServiceException;
import io.hops.hopsworks.persistence.entity.command.Operation;
import io.hops.hopsworks.persistence.entity.command.SystemCommand;
Expand Down Expand Up @@ -74,13 +75,12 @@ public abstract class DockerRegistryMngrImpl implements DockerRegistryMngr {
@EJB
private HttpClient httpClient;


@Override
public final void gc() throws IOException, ServiceException {
public void gc() throws IOException, ServiceException, ProjectException {
// 1. Get all conda commands of type REMOVE. Should be only 1 REMOVE per project
final List<CondaCommands> condaCommandsRemove =
condaCommandFacade.findByStatusAndCondaOp(CondaStatus.NEW,
CondaOp.REMOVE);
final List<CondaCommands> condaCommandsRemove = condaCommandFacade.findByStatusAndCondaOp(CondaStatus.NEW,
CondaOp.REMOVE);
LOG.log(Level.FINE, "condaCommandsRemove: " + condaCommandsRemove);
try {
for (CondaCommands cc : condaCommandsRemove) {
// We do not want to remove the base image! Get arguments from command as project may have already been deleted.
Expand Down Expand Up @@ -109,7 +109,7 @@ public final void gc() throws IOException, ServiceException {
cc.getArg(), cc.getOp(),
"Could not complete docker registry cleanup: " +
ex.getMessage());
} catch (ServiceException e) {
} catch (ServiceException | ProjectException e) {
LOG.log(Level.WARNING,
"Could not change conda command status to NEW.", e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Date;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -122,12 +123,21 @@ public void synchronizeDependencies(Project project, boolean createBaseEnv) thro

@TransactionAttribute(TransactionAttributeType.REQUIRES_NEW)
public void createProjectDockerImage(Project project, Users user) {
// Check if there is no pending CREATE op for this project
List<CondaStatus> statuses = new ArrayList<>();
statuses.add(CondaStatus.NEW);
statuses.add(CondaStatus.ONGOING);
if(!condaCommandFacade.findByStatusAndCondaOpAndProject(statuses, CondaOp.CREATE, project).isEmpty()) {
LOGGER.log(Level.INFO, "There is already a " + CondaOp.CREATE.name() + " operation for this project.");
return;
}
condaEnvironmentOp(CondaOp.CREATE, project.getPythonVersion(), project, user,
project.getPythonVersion(), null, false);
project.setConda(true);
project.setPythonVersion(settings.getDockerBaseImagePythonVersion());
project.setDockerImage(settings.getBaseDockerImagePythonName());
projectFacade.update(project);
projectFacade.flushEm();
}

@TransactionAttribute(TransactionAttributeType.REQUIRES_NEW)
Expand Down Expand Up @@ -156,6 +166,7 @@ public void removeEnvironment(Project project) {
project.setConda(false);
project.setPythonVersion(null);
projectFacade.update(project);
projectFacade.flushEm();
}

/**
Expand Down Expand Up @@ -186,7 +197,13 @@ public void condaEnvironmentRemove(Project project, Users user) {
+ " for project: " + project.getName());
return;
}

List<CondaStatus> statuses = new ArrayList<>();
statuses.add(CondaStatus.NEW);
statuses.add(CondaStatus.ONGOING);
if(!condaCommandFacade.findByStatusAndCondaOpAndProject(statuses, CondaOp.REMOVE, project).isEmpty()) {
LOGGER.log(Level.INFO, "There is already a " + CondaOp.REMOVE.name() + " operation for this project.");
return;
}
condaEnvironmentOp(CondaOp.REMOVE, "", project, user,
project.getDockerImage(), null, false);
}
Expand Down Expand Up @@ -220,6 +237,13 @@ public String[] exportEnv(Project project, Users user, String projectRelativeExp

public void createEnv(Project project, boolean createBaseEnv) throws PythonException,
ServiceException {
List<CondaStatus> statuses = new ArrayList<>();
statuses.add(CondaStatus.NEW);
statuses.add(CondaStatus.ONGOING);
if(!condaCommandFacade.findByStatusAndCondaOpAndProject(statuses,
CondaOp.CREATE, project).isEmpty()) {
throw new PythonException(RESTCodes.PythonErrorCode.ANACONDA_ENVIRONMENT_INITIALIZING, Level.INFO);
}
if (project.getConda()) {
throw new PythonException(RESTCodes.PythonErrorCode.ANACONDA_ENVIRONMENT_ALREADY_INITIALIZED, Level.FINE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public void addPythonDepsForProject(Project proj, Collection<PythonDep> pythonDe
}
proj.setPythonDepCollection(depsInProj);
projectFacade.update(proj);
projectFacade.flushEm();
}

public PythonDep addLibrary(Project proj, Users user, CondaInstallType installType, String channelUrl,
Expand Down
Loading

0 comments on commit 761e7de

Please sign in to comment.