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

[15.10] Fix a bug in IEs when proxying the proxy #1076

Merged

Conversation

natefoo
Copy link
Member

@natefoo natefoo commented Nov 12, 2015

Also, make use of the /import volume conditional.

conditional. Also, fix a bug when using a proxy for the node proxy and
cookie_path is /
@bgruening
Copy link
Member

@natefoo I was able to build bgruening/docker-ipython-notebook:15.10.1 can you please try this on test?

Adding a little bit of documentation:
For IE's that don't need a default volume mounted to /import, like IPython in this version, you can turn it of by using ie_request.attr.import_volume = False in your IE mako.

@natefoo
Copy link
Member Author

natefoo commented Nov 13, 2015

@bgruening Working on Test w/ bgruening/docker-ipython-notebook:15.10.1

BTW, was the problem with Docker Hub really the - in the tag?

@bgruening
Copy link
Member

There was two things one bug in Docker hub that was fixed yesterday and the - yes.
👍 Let's get this in.

martenson added a commit that referenced this pull request Nov 13, 2015
[15.10] Fix a bug in IEs when proxying the proxy
@martenson martenson merged commit 64b0840 into galaxyproject:release_15.10 Nov 13, 2015

if hda.datatype.__class__.__name__ != "Ipynb":
with open( empty_nb_path, 'w+' ) as handle:
handle.write( empty_nb )
Copy link
Member

Choose a reason for hiding this comment

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

Glad to see this removed! Good to have it part of the image. :)

Copy link
Member

Choose a reason for hiding this comment

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

Oh, it isn't part of the image yet. @bgruening let's add a default notebook to the image?

Copy link
Member

Choose a reason for hiding this comment

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

Strike that, I was looking at the master branch. We should probably update master eventually? Also why 15.10.1 instead of just updating 15.10?

Copy link
Member

Choose a reason for hiding this comment

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

Also I know realise we're in november, and fully understand why you did 15.10.1 :) thanks for doing the right thing @bgruening

Copy link
Member

Choose a reason for hiding this comment

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

For people that are already using 15.10, just a way to prevent any harm.
Master is updated as soon as 15.10 is out.

Copy link
Member

Choose a reason for hiding this comment

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

yep :)

@hexylena hexylena added this to the 16.01 milestone Nov 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants