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

Allow Relative Path Urls #28

Closed
DeanPoulin opened this issue Jan 16, 2014 · 8 comments · Fixed by #29
Closed

Allow Relative Path Urls #28

DeanPoulin opened this issue Jan 16, 2014 · 8 comments · Fixed by #29

Comments

@DeanPoulin
Copy link

We currently use grunt with grunt-connect-proxy for local development so that we can code our front end to use relative path AJAX requests to avoid cross site request issues.

When we deploy the application to other environments we simply configure the load balancer to perform the same type of proxying of relative path requests to other servers/ports server side.

I'm attempting to configure the esFactory as such:

angular.module('components.search')
  .service('es', function (esFactory) {
    return esFactory({
      host: '/services/elastic'
    });
  });

and using elasticsearch.angular.js but the AJAX requests are being made to http://localhost/services/elastic instead of /services/elastic

This is causing collisions with the way grunt runs the application in development on a url like http://localhost:9000/#/

Can we update the code to not assume I want to prefix the AJAX requests with http://localhost if I specify a host with a a leading slash?

This might just be achievable through a new configuration param:

angular.module('components.search')
  .service('es', function (esFactory) {
    return esFactory({
      host: '/services/elastic',
      relative: true
    });
  });

and specifically handled in elasticsearch.angular.js when making the call with $http

 this.$http({
    ...
    url: this.host.makeUrl(params),
    ...
  })
@spalger
Copy link
Contributor

spalger commented Jan 16, 2014

While allowing relative paths in the browser certainly makes sense, it would be broken by the cluster discovery functionality. Maybe you should try this approach instead:

angular.module('components.search')
  .service('es', function (esFactory) {
    return esFactory({
      host: window.location.protocol + '//' + window.location.host + '/services/elastic'
    });
  });

@DeanPoulin
Copy link
Author

Well, that doesn't allow us to use grunt-connect-proxy (https://github.com/drewzboto/grunt-connect-proxy). Grunt connect proxy is specifically looking for relative path requests made through $http which it then reroutes through grunt (node) server side to the actual web server configured in grunt.

Your suggestion is causing http requests to http://localhost/services/elastic instead of http://localhost:9000/services/elastic. I'll try a couple other things and report what's happening while I debug through the code.

@DeanPoulin
Copy link
Author

As far as I know we're using this library to just query elastic search (http://elasticsearch.github.io/elasticsearch-js/api.html#search). Perhaps we can just use raw $http requests with the elastic dsl in the http request body.

@spalger
Copy link
Contributor

spalger commented Jan 16, 2014

That's a fair observation. If you are only doing search queries, and your entire cluster is behind a load balancer, then using elasticsearch.js in the browser is probably overkill.

@spalger
Copy link
Contributor

spalger commented Jan 16, 2014

Not sure why I didn't think about this earlier, but you do something like this:

return esFactory({
  host: {
    host: '',
    port: '',
    path: '/services/elastic'
  }
});

right now this will cause the requests to go to http:///services/elastic but it should go to /services/elastic. I'll get that bug patched

@boneill42
Copy link

Nice, great thinking @spenceralger.

@spalger
Copy link
Contributor

spalger commented Jan 16, 2014

@DeanPoulin Just updated the master build, if you want to give it a shot. Check out the unit tests for an example.

@DeanPoulin
Copy link
Author

@spenceralger thanks for your help, I'll give it a spin and let you know how it goes. Much appreciated.

cdituri pushed a commit to cdituri/elasticsearch-js that referenced this issue Jun 14, 2017
fixed undefined is not function issue for expires
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 a pull request may close this issue.

3 participants