这是indexloc提供的服务，不要输入任何密码
Skip to content

serve apis at a root other than / (close #1141) #1171

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 9 commits into from
Closed

serve apis at a root other than / (close #1141) #1171

wants to merge 9 commits into from

Conversation

@rakeshkky
Copy link
Member

@rakeshkky rakeshkky commented Dec 7, 2018

Description

What component does this PR affect?

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Requires changes from other components? If yes, please mark the components:

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Related Issue

close #1141

Solution and Design

Allow server APIs and console to be served at a path other than /
configurable via --url-root command line flag and
HASURA_GRAPHQL_URL_ROOT environment variable

Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Docs update
  • Community content

Checklist:

  • I have read the contributing guide and my code conforms to the guidelines.
  • This change requires a change in the documentation.
  • I have updated the documentation accordingly.
  • I have added required tests.

@rakeshkky rakeshkky added s/ok-to-merge Status: This pull request can be merged to master c/server Related to server labels Dec 7, 2018
@rakeshkky rakeshkky self-assigned this Dec 7, 2018
@rakeshkky rakeshkky requested review from 0x777 and shahidhk December 7, 2018 11:43
@shahidhk shahidhk changed the title allow APIs and console be served at a path other than '/', close #1141 serve apis at a root other than / (close #1141) Dec 7, 2018
@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 7, 2018

Review app for commit c89d023 deployed to Heroku: https://hge-ci-pull-1171.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1171-c89d023

@shahidhk
Copy link
Member

shahidhk commented Dec 7, 2018

@rakeshkky, I set --url-root hello. Now, if I go to /, I get redirected to /hello/console, which is okay, I guess.

But if I go to /hello I get 404. /hello should redirect to /hello/console -- basically it should issue 301 console

@shahidhk shahidhk added c/console Related to console s/do-not-merge Do not merge this pull request to master and removed s/ok-to-merge Status: This pull request can be merged to master labels Dec 7, 2018
@shahidhk
Copy link
Member

shahidhk commented Dec 7, 2018

@praveenweb Looks like the console is not respecting the prefix when generating the API url.

@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 11, 2018

Review app for commit d971da0 deployed to Heroku: https://hge-ci-pull-1171.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1171-d971da0

@danieldunderfelt
Copy link

danieldunderfelt commented Dec 12, 2018 •
edited
Loading

Awesome to see that this is on the way, as this issue is blocking our use of Hasura on a server with many projects on it!

@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 12, 2018

Review app for commit 1b71f3a deployed to Heroku: https://hge-ci-pull-1171.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1171-1b71f3a

karthikvt26
karthikvt26 previously approved these changes Dec 12, 2018
View reviewed changes
Copy link
Contributor

@karthikvt26 karthikvt26 left a comment

Choose a reason for hiding this comment

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

Changes to console code looks fine.

@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 13, 2018

Review app for commit d6cf891 deployed to Heroku: https://hge-ci-pull-1171.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1171-d6cf891

shahidhk
shahidhk previously approved these changes Dec 14, 2018
View reviewed changes
Copy link
Member

@shahidhk shahidhk left a comment

Choose a reason for hiding this comment

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

Functionality is okay. Tested out by putting GraphQL Engine behind caddy.

@rakeshkky Resolve the conflicts and we can merge. @0x777 Awaiting your review.

@rakeshkky
Merge branch 'master' into issue-1141-url-root
3812d6d 
Resolve Conflicts:
	server/src-exec/Main.hs
	server/src-lib/Hasura/Server/Init.hs
@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 14, 2018

Review app for commit 3812d6d deployed to Heroku: https://hge-ci-pull-1171.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1171-3812d6d

@shahidhk shahidhk added this to the v1.0.0-alpha32 milestone Dec 14, 2018
@shahidhk shahidhk added s/ok-to-merge Status: This pull request can be merged to master and removed s/do-not-merge Do not merge this pull request to master labels Dec 15, 2018
@shahidhk
Copy link
Member

shahidhk commented Dec 17, 2018

ref #1141 (comment) for more info

@0x777
Copy link
Member

0x777 commented Dec 17, 2018

closing this in favor of #1222

@0x777 0x777 closed this Dec 17, 2018
@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 17, 2018

Review app https://hge-ci-pull-1171.herokuapp.com is deleted

@cusspvz
Copy link
Contributor

cusspvz commented Oct 27, 2020

@0x777 having some option that allows us to set a prefix on all the server's endpoints would def bring value to Hasura.

As explained at this comment, whenever someone wants to expose hasura on a different path, we need to either get a load-balancer capable of trimming a prefix off the path or place an nginx service that does that.

It seems that this was already done in this effort/PR, but it wasn't merged because the need for this was fixed by other PR.

@0x777 Is there a plan to include an option on the graphql-server similar to this one?

hasura-bot pushed a commit that referenced this pull request Sep 30, 2024
@danieljharvey @hasura-bot
Move model aggregate planning to 'plan' crate (#1171)
a8deb88 
<!-- The PR description should answer 2 important questions: -->

### What

Much in the vein of hasura/v3-engine#1166, we
move the model aggregate planning from `sql` to the `plan` crate. No
tests actually exercise this code in the OpenDD IR pipeline yet, perhaps
if we extend the GraphQL -> OpenDD IR pipeline we can put it under test.

### How

Move the code, fix the errors. Functional no-op.

V3_GIT_ORIGIN_REV_ID: 7beee0aec828296fefa24c975d4662a20aa0d2e5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shahidhk shahidhk shahidhk left review comments

@karthikvt26 karthikvt26 karthikvt26 left review comments

@0x777 0x777 Awaiting requested review from 0x777

Assignees

@rakeshkky rakeshkky

Labels

c/console Related to console c/server Related to server s/ok-to-merge Status: This pull request can be merged to master

Projects

None yet

Milestone

v1.0.0-alpha32

Development

Successfully merging this pull request may close these issues.

expose apis and console at a path other than /

8 participants

@rakeshkky @hasura-bot @shahidhk @danieldunderfelt @0x777 @cusspvz @karthikvt26 @wawhal