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

Core, CORS: allow (non-preflighted) CORS requests by echoing back the… #1331

Merged
merged 8 commits into from Jan 15, 2016
Merged

Core, CORS: allow (non-preflighted) CORS requests by echoing back the… #1331

merged 8 commits into from Jan 15, 2016

Conversation

carlfeberhard
Copy link
Contributor

… origin header if the requesting client is allowed

Fixes #1029

… origin header if the requesting client is allowed
@hexylena hexylena self-assigned this Jan 5, 2016
@hexylena
Copy link
Member

hexylena commented Jan 5, 2016

@galaxybot test this

@carlfeberhard carlfeberhard removed this from the 16.01 milestone Jan 5, 2016
@hexylena
Copy link
Member

hexylena commented Jan 5, 2016

Looks like a failed TS test

Error - <type 'exceptions.AttributeError'>: 'Configuration' object has no attribute 'allowed_origin_hostnames'
URL: http://localhost:8397/
File '/galaxy/lib/galaxy/web/framework/middleware/error.py', line 151 in __call__
  app_iter = self.application(environ, sr_checker)
File '/galaxy/.venv/local/lib/python2.7/site-packages/paste/recursive.py', line 85 in __call__
  return self.application(environ, start_response)
File '/galaxy/.venv/local/lib/python2.7/site-packages/paste/httpexceptions.py', line 640 in __call__
  return self.application(environ, start_response)
File '/galaxy/lib/galaxy/web/framework/base.py', line 127 in __call__
  return self.handle_request( environ, start_response )
File '/galaxy/lib/galaxy/web/framework/base.py', line 157 in handle_request
  self.set_cors_headers( trans )
File '/galaxy/lib/galaxy/web/framework/base.py', line 219 in set_cors_headers
  if not trans.app.config.allowed_origin_hostnames:
AttributeError: 'Configuration' object has no attribute 'allowed_origin_hostnames'

…pplication, bail on lack of config key, rework tests, add test files
@carlfeberhard
Copy link
Contributor Author

Not sure what's going on with Travis here. Can't reproduce this locally with tox either. /shrug

@hexylena
Copy link
Member

hexylena commented Jan 9, 2016

Yeah, dunno about travis either, but I'm a bit nervous to break the build for everyone else.
You have my 👍, but I'd like a second pair of eyes merging this.

@hexylena hexylena removed their assignment Jan 9, 2016
@hexylena
Copy link
Member

hexylena commented Jan 9, 2016

@martenson maybe? You're brave :)

@jmchilton
Copy link
Member

I imagine the Travis problem is this line in Configuration.

        self.new_file_path = resolve_path( kwargs.get( "new_file_path", "database/tmp" ), self.root )
        tempfile.tempdir = self.new_file_path

Invoking a Configuration object changes global state of the Python interpreter for all the other unit tests run in. I'd suggest a flag to configuration to disable that for unit tests.

@carlfeberhard
Copy link
Contributor Author

@jmchilton Interesting. Thanks for the info - I'll check it out

…n instances as it persists across tests and fails any tempfile related tests that follow, move test_config tests into test_webapp (cohesive subject v. ortho file struct)
@carlfeberhard
Copy link
Contributor Author

That was it. Thanks, @jmchilton

jmchilton added a commit that referenced this pull request Jan 15, 2016
Core, CORS: allow (non-preflighted) CORS requests by echoing back the…
@jmchilton jmchilton merged commit 274af50 into galaxyproject:dev Jan 15, 2016
@jmchilton
Copy link
Member

Awesome sauce, glad that worked out. Thanks @carlfeberhard!

@carlfeberhard carlfeberhard added this to the 16.04 milestone Jan 15, 2016
@carlfeberhard carlfeberhard deleted the CORS branch January 15, 2016 16:10
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

3 participants