Scottx611x/read input from url #7
Conversation
Dockerfile
Outdated
| COPY index.html /home/higlass/projects/higlass-website/index.html | ||
|
|
||
| # Swap the "app" html with the main html to always provide the /app/ view | ||
| RUN cp /home/higlass/projects/higlass-website/app/index.html /home/higlass/projects/higlass-website/index.html |
There was a problem hiding this comment.
Would you just want to do a batch copy and maybe get the static assets, etc?
There was a problem hiding this comment.
Oh, this is a hack to only allow the HiGlass /app view i.e. the fullscreen mode. Just overwriting the main index.html, static assets are already in the proper places at this point
on_startup.py
Outdated
|
|
||
|
|
||
| def get_refinery_input_as_json(): | ||
| return requests.get(os.environ["INPUT_JSON_URL"]).json() |
There was a problem hiding this comment.
Generally, we don't want to litter the code with error checking, but this might be an ok place, since we know there are other modes, and better feedback might be valuable... maybe getattr and then make sure it's not None?
on_startup.py
Outdated
| TILE_SETS = {} | ||
|
|
||
|
|
||
| def get_refinery_input_as_json(): |
There was a problem hiding this comment.
Maybe get rid of _as_json: Just reading the method name, I might think this returns a string containing json.
on_startup.py
Outdated
| config_data = get_refinery_input_as_json() | ||
|
|
||
| for refinery_node_uuid in config_data["node_info"]: | ||
| TILE_SETS[refinery_node_uuid] = {} |
There was a problem hiding this comment.
Feels weird to assign to something that should be a constant?
on_startup.py
Outdated
| if HIGLASS_DATA_TYPE in key.lower(): | ||
| TILE_SETS[refinery_node_uuid][HIGLASS_DATA_TYPE] = ( | ||
| refinery_node["node_solr_info"][key] | ||
| ) |
There was a problem hiding this comment.
Maybe a comment about what typical values for keys are? I mistrust the in key.lower(). What behavior do you want if they're both true?
on_startup.py
Outdated
| refinery_node = config_data["node_info"][refinery_node_uuid] | ||
| TILE_SETS[refinery_node_uuid]["file_url"] = refinery_node["file_url"] | ||
| TILE_SETS[refinery_node_uuid]["file_name"] = refinery_node[ | ||
| "file_url"].split("/")[-1] |
There was a problem hiding this comment.
Might be more clear to make a dict and then make one assignment to TILE_SETS[refinery_node_uuid]
on_startup.py
Outdated
| data_dir, | ||
| TILE_SETS[refinery_node_uuid]["file_name"] | ||
| ), 'wb' | ||
| ) as f: |
There was a problem hiding this comment.
Here and above, I think it was more clear having a short variable instead of TILE_SETS[refinery_node_uuid]["file_name"] and TILE_SETS[refinery_node_uuid]["file_url"]
on_startup.py
Outdated
| """ | ||
| for refinery_node_uuid in TILE_SETS.keys(): | ||
| refinery_node_info = TILE_SETS[refinery_node_uuid] | ||
| file_path = data_dir + refinery_node_info["file_name"] |
There was a problem hiding this comment.
This comes goes into filename: Double check? changing kwarg would be out of line?
on_startup.py
Outdated
| # Don't switch page until data ingested | ||
| swap_waiting_page() | ||
| # Start Nginx only after tilesets have been ingested | ||
| subprocess.run(["/usr/sbin/nginx"]) |
There was a problem hiding this comment.
You sort of need to jump through hoops to completely hand off to a new process, but perhaps consider: https://stackoverflow.com/questions/3516007/run-process-and-dont-wait
(In perl exec did this, so I was wondering if there were an analog in python.)
There was a problem hiding this comment.
Comment that parent will hang around
| _ip=${_line[${#_line[1]}>4?1:2]} && | ||
| [ "${_ip#127.0.0.1}" ] && echo http://$_ip:$PYTHON_SERVER_PORT && return 0 | ||
| done< <(LANG=C /sbin/ifconfig) | ||
| } |
There was a problem hiding this comment.
Explain this in plain english and point to SO
test_runner.sh
Outdated
|
|
||
| python -m SimpleHTTPServer $PYTHON_SERVER_PORT & | ||
| PYTHON_SERVER_PID=$! | ||
| docker run --env INPUT_JSON_URL=$(get_local_url)/test-data/input.json \ |
There was a problem hiding this comment.
This is all just to get a static url? Maybe just reference a raw github resource?
tests.py
Outdated
| if 0 == subprocess.call( | ||
| 'curl --fail --silent ' + self.tilesets_url + ' > /dev/null', | ||
| shell=True | ||
| ): |
There was a problem hiding this comment.
I'm embarrassed to have written this code. See https://github.com/refinery-platform/refinery-developer-vis-tool/blob/master/test.py#L26-L42
tests.py
Outdated
|
|
||
| def assertRun(self, command, res=[r'']): | ||
| output = subprocess.check_output(command.format(**os.environ), shell=True).strip() | ||
| def assert_run(self, command, res=[r'']): |
There was a problem hiding this comment.
Does this actually get used in a real test?
…pass values around in global scope
Dockerfile
Outdated
|
|
||
| # Append to the supervisord.conf and set the priority of `on_startup.py` to | ||
| # be greater than the default of `999` so that it starts up last | ||
| RUN ( echo ""; \ |
on_startup.py
Outdated
| # Don't switch page until data ingested | ||
| swap_waiting_page() | ||
| # Start Nginx only after tilesets have been ingested | ||
| subprocess.run(["/usr/sbin/nginx"]) |
There was a problem hiding this comment.
Comment that parent will hang around
| _ip=${_line[${#_line[1]}>4?1:2]} && | ||
| [ "${_ip#127.0.0.1}" ] && echo http://$_ip:$PYTHON_SERVER_PORT && return 0 | ||
| done< <(LANG=C /sbin/ifconfig) | ||
| } |
There was a problem hiding this comment.
Explain this in plain english and point to SO
tests.py
Outdated
| server_check_count = 0 | ||
| while server_check_count <= 5: | ||
| if 0 == subprocess.call( | ||
| 'curl --fail --silent ' + self.tilesets_url + ' > /dev/null', |
tests.py
Outdated
| break | ||
| print('still waiting for server...') | ||
| time.sleep(1) | ||
| time.sleep(5) |
tests.py
Outdated
|
|
||
| def test_home_page(self): | ||
| response = requests.get(self.base_url) | ||
| self.assertEqual(response.status_code, 200) |
There was a problem hiding this comment.
stronger assertions about the homepage
tests.py
Outdated
|
|
||
| # Test if the data we specify in input.json gets ingested properly by | ||
| # Test if the data we specify in INPUT_JSON_URL gets ingested properly by | ||
| # higlass server upon container startup | ||
| def test_data_ingested(self): | ||
| time.sleep(5) |
tests.py
Outdated
| @@ -52,4 +65,3 @@ def test_data_ingested(self): | |||
| print('PASS!') | |||
| else: | |||
| print('FAIL!') | |||
| exit(1) | |||
There was a problem hiding this comment.
Definitely need this, maybe its worth moving simplehttpserver start & cleanup into python
@mccalluc This allows reading from
INPUT_JSON_URL, not bringing up the nginx until tilesets have been ingested, and some general tidying.Tests pass, but there are still only a subset of tracks selectable in the ui which I'll talk to pete about.
Just wanted some feedback before this gets too far along.
Fix #3
Fix #6
Dependent on: refinery-platform/refinery-platform#2677
TODO:
**EDIT: It wasn't feasible to infer HiGlass filetypes/datatypes in an exhaustive manner. So we are only going to accept multires cooler files just as the 4DN team is currently doing.