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 docker image #126

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

harobed
Copy link

@harobed harobed commented Jul 25, 2019

No description provided.

@harobed
Copy link
Author

harobed commented Jul 25, 2019

@IvanGoncharov can you create graphql-voyager account on https://hub.docker.com/ ?

Otherwise, if you want, you can grant me access to https://github.com/APIs-guru/graphql-voyager project and I can maintain Graphql-voyager Docker Image for you ?

README.md Outdated Show resolved Hide resolved
@@ -49,8 +49,7 @@ export default function renderVoyagerPage(options: MiddlewareOptions) {
'Accept': 'application/json',
'Content-Type': 'application/json',
}, ${headersJS}),
body: JSON.stringify({query: introspectionQuery }),
credentials: 'include',
Copy link
Member

Choose a reason for hiding this comment

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

What's the problem with credentials?

Copy link
Author

Choose a reason for hiding this comment

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

@@ -50,8 +50,7 @@
'Accept': 'application/json',
'Content-Type': 'application/json',
},
body: JSON.stringify({query: introspectionQuery}),
credentials: 'include',
Copy link
Member

Choose a reason for hiding this comment

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

same question, why do you delete it?


ENV PORT=3001

CMD node index.js
Copy link
Member

Choose a reason for hiding this comment

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

missing newline at the end of the file.

Copy link
Member

Choose a reason for hiding this comment

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

I actually like to have Dockerfile similar to what graphql-faker has:
https://github.com/APIs-guru/graphql-faker/blob/master/Dockerfile
Is it possible?

Copy link
Member

Choose a reason for hiding this comment

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

Especially since it doesn't require a separate folder and package.json.

Copy link
Author

Choose a reason for hiding this comment

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

missing newline at the end of the file.

Fixed.

Copy link
Author

Choose a reason for hiding this comment

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

Especially since it doesn't require a separate folder and package.json.

I need a place to store index.html and this package.json:

{
  "scripts": {
    "start": "node index.js"
  },
  "dependencies": {
    "express": "^4.16.3",
    "graphql-voyager": "../"
  }
}

I don't want put the mess in root folder.

@IvanGoncharov What do you think of that?

@IvanGoncharov
Copy link
Member

can you create graphql-voyager account on hub.docker.com ?

@harobed We already have account here:
https://cloud.docker.com/repository/docker/apisguru/graphql-faker
So it should be released as apisguru/graphql-voyager

@harobed
Copy link
Author

harobed commented Jul 27, 2019

@harobed We already have account here:
https://cloud.docker.com/repository/docker/apisguru/graphql-faker
So it should be released as apisguru/graphql-voyager

Ok, I have updated Docker image name 👍

@IvanGoncharov
Copy link
Member

@harobed Thanks for PR update. At the moment I'm working on new release of graphql-js but I will try to look into this PR and #127 over weekends.

@harobed
Copy link
Author

harobed commented Aug 1, 2019

Note: when all will be ok, I will squash MR commits.

@Its-Alex
Copy link

@IvanGoncharov up

@harobed
Copy link
Author

harobed commented Nov 6, 2019

@IvanGoncharov how can I help you?

@IvanGoncharov
Copy link
Member

@harobed Sorry pretty busy month.
I need to prepare for the GraphQL Working Group on Thursday but it will be at the top of my queue afterward.
Fun fact: I was at the local OWASP workshop about GraphQL security and the speaker prepared some Docker images including one for Voyager.

@DJ92
Copy link

DJ92 commented Nov 11, 2019

hey guys, was there a resolve for #127 ? Trying to understand why credentials: include would be a default option for the fetch.

@Its-Alex
Copy link

@IvanGoncharov Up ? This would be awesome to use it in a docker directly


WORKDIR /graphql-voyager/

WORKDIR /graphql-voyager/docker-image/
Copy link

Choose a reason for hiding this comment

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

This line makes the previous one unnecessary

@linghengqian
Copy link

Great, hope to see this PR pushed.

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.

7 participants