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

Fix for EtherCalc IE image specification. #3825

Merged
merged 1 commit into from Mar 28, 2017

Conversation

Projects
None yet
4 participants
@blankenberg
Copy link
Member

commented Mar 27, 2017

Will allow EtherCalc to work in 17.05.

Fix for EtherCalc IE image specification.
Will allow EtherCalc to work in 17.05.

@blankenberg blankenberg added this to the 17.05 milestone Mar 27, 2017

@jmchilton jmchilton merged commit 07be669 into galaxyproject:dev Mar 28, 2017

5 checks passed

api test Build finished. 267 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 140 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 25 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 580 tests run, 0 skipped, 0 failed.
Details
@erasche

This comment has been minimized.

Copy link
Member

commented Mar 28, 2017

I'm not completely sure about this. The visualization tab view of GIEs (i.e. allowed_images) is more focused on multiple dataset selection. Ethercalc can't support loading N datasets currently, so moving it there might be a mistake as people might try to select multiple files and expect those to be available.

(Or maybe I'm forgetting how GIEs work in 17.05? and allowed_images is now required?)

@blankenberg

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2017

EtherCalc didn't appear at all for selection from a dataset in history or the IE tab, under a default GIE setup (simple localhost:8080 with node proxy) after copying the .ini, until I made the allowed_images.yml.sample changes.

Also, FWIIW, while the App loads, it always has an empty spreadsheet.

@erasche

This comment has been minimized.

Copy link
Member

commented Mar 28, 2017

@blankenberg that's odd, it appeared for me under datasets in history during PR testing. Maybe we have changed that and I haven't been following along.

It shouldn't have an empty spreadsheet when running under not localhost. I'm looking into those issues, but they didn't seem to be ethercalc issues, more with how we're launching notebooks and passing env vars which configure the galaxy helper clients in the container.

@blankenberg blankenberg deleted the blankenberg:ethercalc branch Mar 28, 2017

@erasche

This comment has been minimized.

Copy link
Member

commented Mar 28, 2017

(Not a personal attack, just investigating.) Just tested, this PR is not required to show up under datasets in history. I reverted the change locally and it works fine and does not show up under the global viz menu but does under dataset viz.

utvalg_108

$ git log
commit cb99052b33c6e24f7d982e4d7189e089e2f106e1
Author: Eric Rasche <rasche.eric@gmail.com>
Date:   Tue Mar 28 16:33:53 2017 +0000

    Revert "Fix for EtherCalc IE image specification."
    
    This reverts commit 21e5731355ebcbba2bb4280844091576c7700abf.

I wanted to make sure I wasn't wrong / didn't merge non-functional code.

Empty on Load ... ugh. This is my least favourite problem. I feel like it used to be easier, and I feel like our magic used to work, but my memory is not good, and the magic has always been dependent on your network configuration, your galaxy config, the container's version of the docker-galaxy utils, a healthy dose of luck, and the phases of the moon.

Personally, I access my galaxy instance at http://leda/galaxy/ and https://leda/galaxy/. This hostname is not passed to the docker container, so ethercalc can't access it. Even if it was, it would only work under http:// since the container would croak on SSL cert errors for the self signed cert. You access yours at localhost, yeah? Then the docker container thinks Galaxy is running on localhost and fails when it isn't there. If I switch to accessing via the public (nat'd) IP of my machine, docker can resolve/route to that address and fetches the data without issue.
utvalg_109

@erasche

This comment has been minimized.

Copy link
Member

commented Mar 28, 2017

I'm testing the jupyter container now (gosh it's massive) to see if the utils are behaving any better in there under my test cases. Will PR ethercalc / raise an issue there if it is behaving differently than jupyter.

@erasche

This comment has been minimized.

Copy link
Member

commented Mar 28, 2017

How saving works while loading fails... well crap. There goes my configuration theory.

@erasche

This comment has been minimized.

Copy link
Member

commented Mar 28, 2017

Opened an issue for myself. Thanks for noticing @blankenberg #3831

@erasche

This comment has been minimized.

Copy link
Member

commented Mar 28, 2017

Ah, yes, I have one of the handful of configurations that will cause a failure for get() but not put(): running on localhost (so not DNS addressable) + X-Sendfile.

@blankenberg can you perhaps elaborate on your setup?

  • do you have galaxy_infrastructure_url set?
  • what does the container environment look like? (Would you mind sharing the launch command / env vars? should show up in the galaxy log if you have the debugging on)
@shiltemann

This comment has been minimized.

Copy link
Member

commented Mar 29, 2017

Hmm, it still works for me without this fix as well ..odd that it's not even appearing in the list for you, having interactive_environment_plugins_directory set in galaxy.ini, a dataset of the correct type (tabular or csv in this case), and being logged in should be enough to at least have it listed as an option as far as I know..

Empty spreadsheets are not surprising when starting it from the launcher, it would require some changes to the docker image to properly load datasets selected by the user in this way, which I would be happy to make, but as @erasche pointed out it cannot handle multiple datasets currently, so I didn't enable it to work with the launcher.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.