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

Add CORS headers to api requests #1870

Merged
merged 2 commits into from Oct 4, 2016
Merged

Conversation

jessicaaustin
Copy link
Contributor

Description

Add the following headers to API server requests:

  • Access-Control-Allow-Headers:Accept, Authorization, Content-Type, Origin
  • Access-Control-Allow-Methods:GET, OPTIONS
  • Access-Control-Allow-Origin:*

Motivation and Context

This allows an external application to request data from the api via Javascript. In our specific case, we request JSON data from /api/graph and display it in an internal ops dashboard.

Have you tested this? If so, how?

Ran the server locally and verified that the desired headers show up. Tested out with an AngularJS application that pulls in luigi task data.

Simple test that you can run in a dev console (requires jQuery): $.get("http://localhost:8082/api/graph", function(data) { console.dir(data.response); })

@mention-bot
Copy link

@jessicaaustin, thanks for your PR! By analyzing the annotation information on this pull request, we identified @ystopia, @erikbern and @freider to be potential reviewers

@jessicaaustin jessicaaustin changed the title Add CORS headers to /api requests Add CORS headers to api requests Sep 30, 2016
@erikbern
Copy link
Contributor

erikbern commented Oct 1, 2016

lgtm!

@Tarrasch
Copy link
Contributor

Tarrasch commented Oct 3, 2016

LGTM. Can you also accompany this patch with test cases?

@jessicaaustin
Copy link
Contributor Author

@Tarrasch test added

successful run: https://travis-ci.org/spotify/luigi/jobs/164703448#L2821

@Tarrasch Tarrasch merged commit 1a5b641 into spotify:master Oct 4, 2016
@Tarrasch
Copy link
Contributor

Tarrasch commented Oct 4, 2016

Awesome!

Is there any chance of open sourcing any of your dashboards? Or at least show screen-shots?

@jessicaaustin
Copy link
Contributor Author

Yep, definitely. I'll post some screenshots at least, but there's a chance at least part of the dashboard will be publicly available. Thanks for merging this PR!

This was referenced Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants