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.

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

Adds http.Server options as configuration server.options #7324

Merged
merged 1 commit into from
Aug 11, 2022
Merged

Adds http.Server options as configuration server.options #7324

merged 1 commit into from
Aug 11, 2022

Conversation

dcousens
Copy link
Member

@dcousens dcousens commented Mar 3, 2022 •
edited
Loading

This pull request adds the http.Server options as an configuration option server.options for your Keystone configuration.

This will enable developers to configuration things like the host interface, or to bind the server to an existing fd descriptor (like if you are using systemd's LISTEN_FDS).

@changeset-bot
Copy link

changeset-bot bot commented Mar 3, 2022 •
edited
Loading

🦋 Changeset detected

Latest commit: 53cfba5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@keystone-6/core Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Mar 3, 2022 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/keystonejs/keystone-next-docs/7LznGhntbm11h22rDxvtyai2HTHo
✅ Preview: https://keystone-next-docs-git-node-http-keystonejs.vercel.app

@dcousens dcousens mentioned this pull request Mar 3, 2022
Closed
@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 3, 2022 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 53cfba5:

Sandbox Source
@keystone-6/sandbox Configuration

@vercel vercel bot temporarily deployed to Preview March 17, 2022 04:48 Inactive
@vercel
Copy link

vercel bot commented May 11, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
keystone-next-docs ✅ Ready (Inspect) Visit Preview Aug 11, 2022 at 4:37AM (UTC)

@vercel vercel bot temporarily deployed to Preview May 11, 2022 02:43 Inactive
@dcousens dcousens force-pushed the node-http branch 2 times, most recently from d120c63 to 1169781 Compare August 11, 2022 04:11
@vercel vercel bot temporarily deployed to Preview August 11, 2022 04:18 Inactive
@dcousens dcousens changed the title Support providing node HTTP options to the http server @dcousens Adds http.Server options as configuration server.options Aug 11, 2022
@dcousens dcousens changed the title @dcousens Adds http.Server options as configuration server.options Adds http.Server options as configuration server.options Aug 11, 2022
@dcousens dcousens marked this pull request as ready for review August 11, 2022 04:19
@dcousens dcousens requested a review from emmatown August 11, 2022 04:30
@vercel
Copy link

vercel bot commented Aug 11, 2022

Deployment failed with the following error:

Invalid request: `target` should be string.

@dcousens dcousens force-pushed the node-http branch 2 times, most recently from 5f052d5 to 53cfba5 Compare August 11, 2022 04:32
@vercel vercel bot temporarily deployed to Preview August 11, 2022 04:37 Inactive
emmatown
emmatown approved these changes Aug 11, 2022
View reviewed changes
Copy link
Member

@emmatown emmatown left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I want to note that this makes process.env.PORT preferred over config.server.port which I agree makes more sense though you could argue that it's breaking but as long as you're aware of that and think that's fine then I'm ok with it.

@dcousens dcousens merged commit 3eaab49 into main Aug 11, 2022
@dcousens dcousens deleted the node-http branch August 11, 2022 05:26
@keystonejs-release-bot keystonejs-release-bot mentioned this pull request Aug 11, 2022
Merged
@dcousens
Copy link
Member Author

dcousens commented Aug 11, 2022 •
edited
Loading

I think the previous behaviour is indicative of broken unexpected behaviour, and I'm happy to fix that as a patch; however I will append that to the changeset for anyone upgrading.

dcousens added a commit that referenced this pull request Aug 11, 2022
@dcousens
add patch changeset for #7324
a438fe9
@dcousens dcousens mentioned this pull request Aug 11, 2022
Merged
dcousens added a commit that referenced this pull request Aug 11, 2022
@dcousens
Add patch changeset for #7324 (#7787)
721ad00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emmatown emmatown emmatown approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dcousens @emmatown