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.

Sign up for GitHub

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

allow writing into Data Volume in job submission #9

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

allow writing into Data Volume in job submission #9

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
346b9ca
allow writing into Data Volume in job submission
mtaghiza Mar 24, 2020
59348ba
add name to data volume definition in job submitting methods
mtaghiza Apr 3, 2020
804b3ce
fixed typo
mtaghiza Apr 3, 2020
b869120
fixed documentation, open jobs, and allowed actions checks
mtaghiza Apr 21, 2020
9d0f19a
added getJobQueues function, and improved way of setting/defining res…
mtaghiza May 12, 2020
127fcac
small fix
mtaghiza May 13, 2020
4781501
allow including all data volumes and user volumes by specifying 'all'…
mtaghiza Mar 9, 2021
6db2371
raise exception with None values for user/data volumes in docker job …
mtaghiza Mar 9, 2021
b2ad78c
include 'all' option in missing remaining methods
mtaghiza Mar 9, 2021
ace43f8
fifed 'all' assignement
mtaghiza Mar 9, 2021
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
20 changes: 9 additions & 11 deletions py2/SciServer/Jobs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def getJobsList(top=10, open=None, start=None, end=None, type='all'):
if(type=='docker'):
url = Config.RacmApiURL + "/jobm/rest/dockerjobs?"

url = url + topString + startString + endString + "TaskName=" + taskName;
url = url + topString + startString + endString + openString + "TaskName=" + taskName;

headers = {'X-Auth-Token': token, "Content-Type": "application/json"}
res = requests.get(url, headers=headers, stream=True)
Expand Down Expand Up @@ -345,7 +345,7 @@ def submitNotebookJob(notebookPath, dockerComputeDomain=None, dockerImageName=No
:param dockerComputeDomain: object (dictionary) that defines a Docker compute domain. A list of these kind of objects available to the user is returned by the function Jobs.getDockerComputeDomains().
:param dockerImageName: name (string) of the Docker image for executing the notebook. E.g., dockerImageName="Python (astro)". An array of available Docker images is defined as the 'images' property in the dockerComputeDomain object.
:param userVolumes: a list with the names of user volumes (with optional write permissions) that will be mounted to the docker Image. E.g.: userVolumes = [{'name':'persistent', 'needsWriteAccess':False},{'name':'scratch', , 'needsWriteAccess':True}] . A list of available user volumes can be found as the 'userVolumes' property in the dockerComputeDomain object. If userVolumes=None, then all available user volumes are mounted, with 'needsWriteAccess' = True if the user has Write permissions on the volume.
:param dataVolumes: a list with the names of data volumes that will be mounted to the docker Image. E.g.: dataVolumes=[{"name":"SDSS_DAS"}, {"name":"Recount"}]. A list of available data volumes can be found as the 'volumes' property in the dockerComputeDomain object. If dataVolumes=None, then all available data volumes are mounted.
:param dataVolumes: a list with the names of data volumes (with optional write permissions) that will be mounted to the docker Image. E.g.: dataVolumes=[{"name":"SDSS_DAS", 'needsWriteAccess':False}, {"name":"Recount"}]. A list of available data volumes can be found as the 'volumes' property in the dockerComputeDomain object. If dataVolumes=None, then all available data volumes are mounted.
:param resultsFolderPath: full path to results folder (string) where the original notebook is copied to and executed. E.g.: /home/idies/workspace/rootVolume/username/userVolume/jobsFolder. If not set, then a default folder will be set automatically.
:param parameters: string containing parameters that the notebook might need during its execution. This string is written in the 'parameters.txt' file in the same directory level where the notebook is being executed.
:param jobAlias: alias (string) of job, defined by the user.
Expand Down Expand Up @@ -410,7 +410,7 @@ def submitNotebookJob(notebookPath, dockerComputeDomain=None, dockerImageName=No
datVols = [];
if dataVolumes is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is not part of this PR, but I believe that when no dataVolumes are explicitly provided, whether as [] or as None, we should NOT mount any data volumes.

Copy link
Contributor

Choose a reason for hiding this comment

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

however, in any case I would NOT make them writable, even though the DV may be writable by default. I would be happy if users are explicit in what thye want us to do, and if by accident (not providing explicit list) they mount all data volumes, at least they should not be writable.

Copy link

Choose a reason for hiding this comment

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

Currently behavior is to mount everything if no volumes are specified though. I think that does make for a much more convenient experience for the user instead having to specify a long list of dictionaries of user and data volumes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jonashaase : could you confirm you agree with the changes I just made?

for vol in dockerComputeDomain.get('volumes'):
if 'write' in vol.get('allowedActions'):
if vol.get('writable') is True:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': True});
else:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': False});
Expand All @@ -422,12 +422,12 @@ def submitNotebookJob(notebookPath, dockerComputeDomain=None, dockerImageName=No
if vol.get('name') == dVol.get('name'):
found = True;
if dVol.get('needsWriteAccess'):
if dVol.get('needsWriteAccess') is True and 'write' in vol.get('allowedActions'):
if dVol.get('needsWriteAccess') is True:
Copy link
Contributor

Choose a reason for hiding this comment

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

why is the constraint on "'write' in allowedActions" removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a bug fix (vol does not have 'allowedActions' field)

datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': True});
else:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': False});
else:
if 'write' in vol.get('allowedActions'):
if vol.get('writable') is True:
Copy link
Contributor

Choose a reason for hiding this comment

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

again here if a user does not ask for write access we should not give it by default, even if they are allowed to write there..
I'd rather err on the side of caution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed that.

datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': True});
else:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': False});
Expand Down Expand Up @@ -471,9 +471,7 @@ def submitShellCommandJob(shellCommand, dockerComputeDomain = None, dockerImageN
:param userVolumes: a list with the names of user volumes (with optional write permissions) that will be mounted to the docker Image.
E.g., userVolumes = [{'name':'persistent', 'needsWriteAccess':False},{'name':'scratch', , 'needsWriteAccess':True}]
A list of available user volumes can be found as the 'userVolumes' property in the dockerComputeDomain object. If userVolumes=None, then all available user volumes are mounted, with 'needsWriteAccess' = True if the user has Write permissions on the volume.
:param dataVolumes: a list with the names of data volumes that will be mounted to the docker Image.
E.g., dataVolumes=[{"name":"SDSS_DAS"}, {"name":"Recount"}].
A list of available data volumes can be found as the 'volumes' property in the dockerComputeDomain object. If dataVolumes=None, then all available data volumes are mounted.
:param dataVolumes: a list with the names of data volumes (with optional write permissions) that will be mounted to the docker Image. E.g.: dataVolumes=[{"name":"SDSS_DAS", 'needsWriteAccess':False}, {"name":"Recount"}]. A list of available data volumes can be found as the 'volumes' property in the dockerComputeDomain object. If dataVolumes=None, then all available data volumes are mounted.
:param resultsFolderPath: full path to results folder (string) where the shell command is executed. E.g.: /home/idies/workspace/rootVolume/username/userVolume/jobsFolder. If not set, then a default folder will be set automatically.
:param jobAlias: alias (string) of job, defined by the user.
:return: the job ID (int)
Expand Down Expand Up @@ -538,7 +536,7 @@ def submitShellCommandJob(shellCommand, dockerComputeDomain = None, dockerImageN
datVols = [];
if dataVolumes is None:
for vol in dockerComputeDomain.get('volumes'):
if 'write' in vol.get('allowedActions'):
if vol.get('writable') is True:
Copy link
Contributor

Choose a reason for hiding this comment

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

see comments above.

datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': True});
else:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': False});
Expand All @@ -550,12 +548,12 @@ def submitShellCommandJob(shellCommand, dockerComputeDomain = None, dockerImageN
if vol.get('name') == dVol.get('name'):
found = True;
if dVol.get('needsWriteAccess'):
if dVol.get('needsWriteAccess') is True and 'write' in vol.get('allowedActions'):
if dVol.get('needsWriteAccess') is True:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': True});
else:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': False});
else:
if 'write' in vol.get('allowedActions'):
if vol.get('writable') is True:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': True});
else:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': False});
Expand Down
22 changes: 11 additions & 11 deletions py3/SciServer/Jobs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def getJobsList(top=10, open=None, start=None, end=None, type='all'):
if(type=='docker'):
url = Config.RacmApiURL + "/jobm/rest/dockerjobs?"

url = url + topString + startString + endString + "TaskName=" + taskName;
url = url + topString + startString + endString + openString + "TaskName=" + taskName;

headers = {'X-Auth-Token': token, "Content-Type": "application/json"}
res = requests.get(url, headers=headers, stream=True)
Expand Down Expand Up @@ -345,7 +345,7 @@ def submitNotebookJob(notebookPath, dockerComputeDomain=None, dockerImageName=No
:param dockerComputeDomain: object (dictionary) that defines a Docker compute domain. A list of these kind of objects available to the user is returned by the function Jobs.getDockerComputeDomains().
:param dockerImageName: name (string) of the Docker image for executing the notebook. E.g., dockerImageName="Python (astro)". An array of available Docker images is defined as the 'images' property in the dockerComputeDomain object.
:param userVolumes: a list with the names of user volumes (with optional write permissions) that will be mounted to the docker Image. E.g.: userVolumes = [{'name':'persistent', 'needsWriteAccess':False},{'name':'scratch', , 'needsWriteAccess':True}] . A list of available user volumes can be found as the 'userVolumes' property in the dockerComputeDomain object. If userVolumes=None, then all available user volumes are mounted, with 'needsWriteAccess' = True if the user has Write permissions on the volume.
:param dataVolumes: a list with the names of data volumes that will be mounted to the docker Image. E.g.: dataVolumes=[{"name":"SDSS_DAS"}, {"name":"Recount"}]. A list of available data volumes can be found as the 'volumes' property in the dockerComputeDomain object. If dataVolumes=None, then all available data volumes are mounted.
:param dataVolumes: a list with the names of data volumes (with optional write permissions) that will be mounted to the docker Image. E.g.: dataVolumes=[{"name":"SDSS_DAS", 'needsWriteAccess':False}, {"name":"Recount"}]. A list of available data volumes can be found as the 'volumes' property in the dockerComputeDomain object. If dataVolumes=None, then all available data volumes are mounted.
:param resultsFolderPath: full path to results folder (string) where the original notebook is copied to and executed. E.g.: /home/idies/workspace/rootVolume/username/userVolume/jobsFolder. If not set, then a default folder will be set automatically.
:param parameters: string containing parameters that the notebook might need during its execution. This string is written in the 'parameters.txt' file in the same directory level where the notebook is being executed.
:param jobAlias: alias (string) of job, defined by the user.
Expand Down Expand Up @@ -410,7 +410,7 @@ def submitNotebookJob(notebookPath, dockerComputeDomain=None, dockerImageName=No
datVols = [];
if dataVolumes is None:
for vol in dockerComputeDomain.get('volumes'):
if 'write' in vol.get('allowedActions'):
if vol.get('writable') is True:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': True});
else:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': False});
Expand All @@ -422,12 +422,12 @@ def submitNotebookJob(notebookPath, dockerComputeDomain=None, dockerImageName=No
if vol.get('name') == dVol.get('name'):
found = True;
if dVol.get('needsWriteAccess'):
if dVol.get('needsWriteAccess') is True and 'write' in vol.get('allowedActions'):
if dVol.get('needsWriteAccess') is True:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': True});
else:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': False});
else:
if 'write' in vol.get('allowedActions'):
if vol.get('writable') is True:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': True});
else:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': False});
Expand Down Expand Up @@ -469,10 +469,10 @@ def submitShellCommandJob(shellCommand, dockerComputeDomain = None, dockerImageN
:param dockerComputeDomain: object (dictionary) that defines a Docker compute domain. A list of these kind of objects available to the user is returned by the function Jobs.getDockerComputeDomains().
:param dockerImageName: name (string) of the Docker image for executing the notebook. E.g., dockerImageName="Python (astro)". An array of available Docker images is defined as the 'images' property in the dockerComputeDomain object.
:param userVolumes: a list with the names of user volumes (with optional write permissions) that will be mounted to the docker Image.
E.g., userVolumes = [{'name':'persistent', 'needsWriteAccess':False},{'name':'scratch', , 'needsWriteAccess':True}]
E.g., userVolumes = [{'name':'persistent', 'needsWriteAccess':False},{'name':'scratch', 'needsWriteAccess':True}]
A list of available user volumes can be found as the 'userVolumes' property in the dockerComputeDomain object. If userVolumes=None, then all available user volumes are mounted, with 'needsWriteAccess' = True if the user has Write permissions on the volume.
:param dataVolumes: a list with the names of data volumes that will be mounted to the docker Image.
E.g., dataVolumes=[{"name":"SDSS_DAS"}, {"name":"Recount"}].
:param dataVolumes: a list with the names of data volumes (with optional write permissions) that will be mounted to the docker Image.
E.g., dataVolumes=[{"name":"SDSS_DAS", 'needsWriteAccess':False}, {"name":"Recount"}].
A list of available data volumes can be found as the 'volumes' property in the dockerComputeDomain object. If dataVolumes=None, then all available data volumes are mounted.
:param resultsFolderPath: full path to results folder (string) where the shell command is executed. E.g.: /home/idies/workspace/rootVolume/username/userVolume/jobsFolder. If not set, then a default folder will be set automatically.
:param jobAlias: alias (string) of job, defined by the user.
Expand Down Expand Up @@ -538,7 +538,7 @@ def submitShellCommandJob(shellCommand, dockerComputeDomain = None, dockerImageN
datVols = [];
if dataVolumes is None:
for vol in dockerComputeDomain.get('volumes'):
if 'write' in vol.get('allowedActions'):
if vol.get('writable') is True:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': True});
else:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': False});
Expand All @@ -550,12 +550,12 @@ def submitShellCommandJob(shellCommand, dockerComputeDomain = None, dockerImageN
if vol.get('name') == dVol.get('name'):
found = True;
if dVol.get('needsWriteAccess'):
if dVol.get('needsWriteAccess') is True and 'write' in vol.get('allowedActions'):
if dVol.get('needsWriteAccess') is True:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': True});
else:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': False});
else:
if 'write' in vol.get('allowedActions'):
if vol.get('writable') is True:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': True});
else:
datVols.append({'id': vol.get('id'), 'name': vol.get('name'), 'writable': False});
Expand Down