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

17.05 rstudio fixes / one small gie fix #3975

Merged
merged 3 commits into from Apr 26, 2017

Conversation

Projects
None yet
3 participants
@erasche
Copy link
Member

commented Apr 25, 2017

  • rstudio ... . I'm disabling the warning on this because it can't be bothered to play nicely with reverse proxies or non-standard ports. Instead it returns a really bad location (http even under https) issuing a security warning despite the fact that rstudio loads and is logged in in the main window. Yay. :|
  • disables the default volume mount for rstudio that wasn't very helpful anyway.
  • the small gie fix is when no volumes are there but additional_datasets are, it throws an error trying to append a list to None. (How did this ever work? I feel like I'm breaking things even worse by doing this and somehow a default volume was supposed to be specified? Really not sure. But this is a "safe" change in that it can't make things worse at least.)

erasche added some commits Apr 25, 2017

@erasche erasche added this to the 17.05 milestone Apr 25, 2017

@erasche erasche requested a review from bgruening Apr 26, 2017

ie_request.launch(
image=trans.request.params.get('image_tag', None),
additional_ids=trans.request.params.get('additional_dataset_ids', None),
env_override={
'notebook_username': USERNAME,
'notebook_password': PASSWORD,
'dataset_hid': DATASET_HID,

This comment has been minimized.

Copy link
@bgruening

bgruening Apr 26, 2017

Member

Do you make use of this?

This comment has been minimized.

Copy link
@erasche

erasche Apr 26, 2017

Author Member

I plan to, since jupyter uses that.

This comment has been minimized.

Copy link
@bgruening

bgruening Apr 26, 2017

Member

Ok, looks good otherwise.
The IE bug is probably due to that people never used it with multiple datasets.
Stil downloading the image ...

@@ -438,6 +438,10 @@ def launch(self, image=None, additional_ids=None, env_override=None, volumes=Non
raise Exception("Attempting to launch disallowed image! %s not in list of allowed images [%s]"
% (image, ', '.join(self.allowed_images)))

# Do not allow a None volumes

This comment has been minimized.

Copy link
@jmchilton

jmchilton Apr 26, 2017

Member

I have this fix in my local work also - definitely needed.

This comment has been minimized.

Copy link
@bgruening

bgruening Apr 26, 2017

Member

Wondering if this is a dev change :(

This comment has been minimized.

Copy link
@jmchilton

jmchilton Apr 26, 2017

Member

Yeah - Nate broke it in a container interface refactor. I had the exact commit earlier but I lost it. No big deal - I don't think it is broken in release_17.01.

@bgruening

This comment has been minimized.

Copy link
Member

commented Apr 26, 2017

Seems to work here. Thanks @erasche!

@bgruening bgruening merged commit 740d85a into galaxyproject:dev Apr 26, 2017

4 of 5 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
api test Build finished. 274 tests run, 0 skipped, 0 failed.
Details
framework test Build finished. 148 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 34 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 579 tests run, 0 skipped, 0 failed.
Details

@bgruening bgruening deleted the erasche:17.05-gie-fixes branch Apr 26, 2017

@erasche

This comment has been minimized.

Copy link
Member Author

commented Apr 26, 2017

awesome, thanks @bgruening.

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.