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

Optional ETag and Content-Format in SetResponse #140

Closed
Tracked by #368
illmade-knight opened this issue Jul 15, 2020 · 5 comments
Closed
Tracked by #368

Optional ETag and Content-Format in SetResponse #140

illmade-knight opened this issue Jul 15, 2020 · 5 comments

Comments

@illmade-knight
Copy link

The ResponseWriter by default adds options for ETag and Content-Format if a body io.ReadSeeker is present.
We're very byte conscious and as neither are required by RFC 7252 it would be preferable if their inclusion was optional.

@niondir
Copy link
Collaborator

niondir commented Dec 27, 2020

I also like the idea and it seems straight forward to implement.

How should the API look like?Just give access to the response object of the response writer?

@niondir
Copy link
Collaborator

niondir commented Dec 28, 2020

You can already send a response without ETag and ContentType when using the Client()

The only remaining side-effect is, that an ACK is send for Confirmable requests and you can not piggyback your response this way.

	m.Handle("/a", mux.HandlerFunc(func(w mux.ResponseWriter, r *mux.Message) {
		assert.Equal(t, codes.GET, r.Code)
		customResp := message.Message{
			Code:    codes.Content,
			Token:   r.Token,
			Context: r.Context,
		}

		err = w.Client().WriteMessage(&customResp)
		if err != nil {
			log.Printf("cannot set response: %v", err)
		}
	}))

@illmade-knight
Copy link
Author

I'll need to dig back into this but I think the Confirmable request was actually the exact problem I was having where I needed to use w.SetResponse(...) instead of w.Client().WriteMessage(...)
In the meantime I've just set a minimal ETag in SetResponse's options to keep the bytes down

@jkralik jkralik mentioned this issue Jul 30, 2022
12 tasks
@JosefWN
Copy link
Contributor

JosefWN commented Jul 31, 2022

Related to: #349

@jkralik
Copy link
Member

jkralik commented Oct 15, 2022

Implemented in v3. Feel free to reopen it. Thx

@jkralik jkralik closed this as completed Oct 15, 2022
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

No branches or pull requests

4 participants