Deprecate NewDefaultServer

[StevenACoffman]

This was only there for tests and as an example, but people have tried to use it in their production environments and found out the hard way it wasn't intended for that. Deprecating will hopefully make this more clear.

However, all the deprecation warnings for all the test usage cause the linter to fail. 😢

I don't know of a great solution, so if you have a suggestion for how to fix this, I would appreciate it.

I'm not sure of a great way to make a test only equivalent function without using a special build constraint tag that would then cause more people to stumble on more regular go test ./...

I could add a //nolint:staticcheck // SA1019: handler.NewDefaultServer is deprecated but usage for test is ok everywhere, but that ignores all staticcheck checks, not just the deprecation one.

I could copy-paste the NewDefaultserver function to be nested inside the various call sites, but that's a lot of duplication.

Signed-off-by: Steve Coffman steve@khanacademy.org

[coveralls]

coverage: 59.668% (-0.2%) from 59.861%
when pulling ca4954c on deprecate_new_default_server
into 3736848 on master.

[phughes-scwx]

What help do you want?

[StevenACoffman]

All the deprecation warnings for all the test usage cause the linter to fail. 😢

I don't know of a great solution, so if you have a suggestion, I would appreciate it.

I'm not sure of a great way to make a test only equivalent function without using a special build constraint tag that would then cause more people to stumble on more regular go test ./...

I could add a //nolint:staticcheck // SA1019: handler.NewDefaultServer is deprecated but usage for test is ok everywhere, but that ignores all staticcheck checks, not just the deprecation one.

I could copy-paste the NewDefaultserver function to be nested inside the various call sites, but that's a lot of duplication.

[phughes-scwx]

I can make a pass over it. I think, if you are deprecating something, you should go ahead and remove it from internal code. I can get a glass of red, listen to a podcast, and start ctrl-v-ing a whole bunch.

[phughes-scwx]

#3406 has this branch as its base, as I can not push directly.

[StevenACoffman]

Thanks again!

[idc77]

How do you propose to replace it and what did "they find out the hard way"?

[StevenACoffman]

@idc77

In the code, the comment for it now tries to explain that you should make your own local version of it, customized to suit your own needs (only add the transports you want, and customize the cache and transports to match your workloads):

// NewDefaultServer is a demonstration only. Not for prod.
//
// Currently, the server just picks the first available transport,
// so this example NewDefaultServer orders them, but it is just
// for demonstration purposes.
// You will likely want to tune and better configure Websocket transport
// since adding a new one (To configure it) doesn't have effect.
//
// Also SSE support is not in here at all!
// SSE when used over HTTP/1.1 (but not HTTP/2 or HTTP/3),
// SSE suffers from a severe limitation to the maximum number
// of open connections of 6 per browser. See:
// https://developer.mozilla.org/en-US/docs/Web/API/Server-sent_events/Using_server-sent_events#sect1
//
// Deprecated: This was and is just an example.
func NewDefaultServer(es graphql.ExecutableSchema) *Server {
	srv := New(es)

	srv.AddTransport(transport.Websocket{
		KeepAlivePingInterval: 10 * time.Second,
	})
	srv.AddTransport(transport.Options{})
	srv.AddTransport(transport.GET{})
	srv.AddTransport(transport.POST{})
	srv.AddTransport(transport.MultipartForm{})

	srv.SetQueryCache(lru.New[*ast.QueryDocument](1000))

	srv.Use(extension.Introspection{})
	srv.Use(extension.AutomaticPersistedQuery{
		Cache: lru.New[string](100),
	})

	return srv
}