Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[WIP] everware.yml with volumes #190 #193

Open
wants to merge 13 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

igormusinov commented Mar 30, 2017

mount files downloaded from into /notebook/data
can't mount in /notebooks (it deletes git files)

@igormusinov igormusinov everware.yml with volumes
mount files downloaded from <url> into /notebook/data
can't mount in /notebooks (it deletes git files)
52d3fa4

igormusinov requested a review from anaderi Mar 30, 2017

everware/git_processor.py
@@ -177,3 +184,30 @@ def load_state(self, state):
if key in state:
setattr(self, key, state[key])
+ def parse(self, everware_yml):
@anaderi

anaderi Mar 31, 2017

Owner

make function name more specific, parse_everware_yml?

everware/git_processor.py
+ text = yaml.load(file)
+ volumes = []
+ for element in text:
+ volumes += text.get(element).get("volumes")
@anaderi

anaderi Mar 31, 2017

Owner

let's rename it into 'data' ('volumes' is already taken)
also it should be possible to specify exact location inside container where data volume should be mapped (or use '/notebook/data' by default)

everware/git_processor.py
+ self.download_file(volume)
+
+ def download_file(self, url):
+ import urllib
@anaderi

anaderi Mar 31, 2017

Owner

it should put some info into log about starting download, file size, etc

everware/spawner.py
@@ -371,6 +373,9 @@ def start(self, image=None):
yield self.remove_old_container()
self.log.info("Starting container from image: %s" % image_name)
self._add_to_log('Creating container')
+ #can't mount into /notebooks folder - it deletes all git files
+ if self.everware_yml_param.get("directory_volume"):
+ self.volumes = {self.everware_yml_param.get("directory_volume") : "/notebook/data"}
@anaderi

anaderi Mar 31, 2017

Owner

directory to map data folder to ('/notebook/data') should be defined inside parse_everware_yml

everware/git_processor.py
+ directory_volume = self._repo_dir + "-volume"
+ os.mkdir(directory_volume)
+ try:
+ urllib.request.urlretrieve(url, directory_volume + '/' + filename)
@betatim

betatim Apr 1, 2017

Owner

I think this call is blocking, so the server will stop being responsive while doing this. Take a look at the OAuthAuthenticator and co for an example of using a non-blocking HTTP client in tornado

@anaderi

needs fixing and tests as well

@@ -1,9 +1,13 @@
import re
import git
from concurrent.futures import ThreadPoolExecutor
-from tornado import gen
+from tornado import gen,concurrent
@anaderi

anaderi Apr 12, 2017

Owner

pep8 (space after comma)

@@ -118,6 +122,34 @@ def prepare_local_repo(self):
else:
return True
+ @gen.coroutine
+ def prepare_everware_yml(self, _user_log):
@anaderi

anaderi Apr 12, 2017

Owner

I'd call it 'parse_repo_everware_yml'

+ for data in all_data:
+ if type(data) == dict:
+ url = data.get("url")
+ ssl = data.get("ssl")
@anaderi

anaderi Apr 12, 2017

Owner

ssl variable should be initialised above

+ setattr(self, key, state[key])
+
+ def download_file(self, url):
@anaderi

anaderi Apr 12, 2017

Owner

why do you need this?

+ try:
+ n_check_connection = 1
+ self.log.info("Downloading from xrootd server %s" % url_struct.hostname)
@anaderi

anaderi Apr 12, 2017

Owner

specify destination as well

+ self.log.info("Downloading from http server %s" % url_struct.hostname)
+ response = yield http_client.fetch(url_struct.geturl())
+ with open(self.directory_data + '/' + filename, "ab") as f:
@anaderi

anaderi Apr 12, 2017

Owner

why mode is "ab"? not "wb"?

+ http_client.close()
+ except:
+ self.log.info("Something went wrong during downloading from %s " % url_struct.hostname)
@anaderi

anaderi Apr 12, 2017

Owner

print exception details here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment