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

[WIP] Explicit proxies #1530

Merged
merged 10 commits into from
Aug 7, 2014
Merged

[WIP] Explicit proxies #1530

merged 10 commits into from
Aug 7, 2014

Conversation

trek
Copy link
Contributor

@trek trek commented Jul 30, 2014

Just a WIP for having more complex proxy behavior.

Proposed changes:

  • renames api-stub generator to http-mock, which is maybe more accurate? :bikeshed:
  • adds an http-proxy with example proxy: ember g proxy api 127.0.0.1:8000
  • updates server directory to include mocks and proxies
  • makes proxies happen last in mocks, middlewares, proxies ordering
  • make proxies with with requests that need to be streams
  • tests

Nice to have

  • leaves --proxy behavior, but warn if used when explicit proxies are generated

@rwjblue
Copy link
Member

rwjblue commented Jul 30, 2014

Definitely 👍 on the idea, ping when its ready for detailed review...

@@ -9,7 +9,7 @@

var bodyParser = require('body-parser');
var globSync = require('glob').sync;
var routes = globSync('./routes/**/*.js', { cwd: __dirname }).map(require);
var routes = globSync('./*/**/*.js', { cwd: __dirname }).map(require);
Copy link
Contributor

Choose a reason for hiding this comment

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

i think the first /* can be dropped /**/*.js should be sufficient

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@EndangeredMassa said that accidentally tried to load server/index.js itself?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, yes it would.

@stefanpenner
Copy link
Contributor

any ideas if proxy should occur before or after other middle ware and if so how would one accomplish this

@trek trek added the proxy label Jul 31, 2014
@stefanpenner
Copy link
Contributor

@trek let me know if I'm blocking you on this one

@trek
Copy link
Contributor Author

trek commented Aug 4, 2014

@stefanpenner I can see problems with both proxy orderings: before and after. Should we just pick one and let it fly? I'd think letting new middleware break apps would be worse than letting proxies break middleware but both seem bad.

Do you have an example of what the middleware do? we've never used one.

@stefanpenner
Copy link
Contributor

@trek can we make proxies just happen last? I suspect if someone overrides a route explicitly they understand and expect that functionality

@trek
Copy link
Contributor Author

trek commented Aug 4, 2014

roger roger.

@stefanpenner
Copy link
Contributor

@trek once you feel good about this, if you can add an integration test (so we can ensure the generator keeps generating something that actually proxies) that would be awesome.

Other then that i think this is good, once you think its good give it a good change-log entry and pull the switch.

@trek
Copy link
Contributor Author

trek commented Aug 5, 2014

not a huge fan of files/server/index.js appearing in both blueprints. Is there some way to share?

@stefanpenner
Copy link
Contributor

@jgwhite thoughts?

@jgwhite
Copy link
Contributor

jgwhite commented Aug 5, 2014

@stefanpenner @trek long-term, taking Thor as inspiration, my feeling is that blueprints should have a set of utility functions along the lines of:

  • Blueprint#addFile(file, location)
  • Blueprint#addPackageToProject(package, version)
  • Blueprint#addRouteToRouter(route, options)
  • Blueprint#installOtherBlueprint(name) (this would have to delegate to the host task for lookup paths)

@trek
Copy link
Contributor Author

trek commented Aug 6, 2014

test failures seem to be install/build based?

@bcardarella
Copy link
Contributor

@trek AppVeyor builds are broken for now

@stefanpenner
Copy link
Contributor

it looks like travis is also broken for the PR: https://travis-ci.org/stefanpenner/ember-cli/pull_requests

@EndangeredMassa
Copy link
Contributor

Green travis-ci build, at least. https://travis-ci.org/stefanpenner/ember-cli/builds/31859850

stefanpenner added a commit that referenced this pull request Aug 7, 2014
@stefanpenner stefanpenner merged commit c65f9c2 into ember-cli:master Aug 7, 2014
@trek trek deleted the explicit-proxies branch September 8, 2014 18:48
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.

6 participants