search

Arksine / moonraker

Web API Server for Klipper
https://moonraker.readthedocs.io
GNU General Public License v3.0
1.02k stars 392 forks source link

spoolman: Include total count header #843

Closed Clon1998 closed 2 months ago

Clon1998 commented 2 months ago

Closes #841.

As noted in the linked issue, spoolman uses the x-total-count header field in its response to indicate the total count, if appropriate, for a pagination result.

To also include this in the proxy I modified the returned JSON of the V2 response to not only return the response but also a response_header field with all custom headers (Headers starting with x-).

I am open to suggestions or alternative ideas on how to handle this change. I feel that adding it as part of the V2 is the safest and will not break any current implementations.

Arksine commented 2 months ago

I can see how this would be useful. I'm wondering if we should convert the full headers object to a dict and return them all. Are we sure we only need the extended headers? The entire headers object may not be necessary, but I don't think it would hurt to return them all either.

Also, when ready to merge, don't forget to add your signed off line. Thanks.

Clon1998 commented 2 months ago

I can see how this would be useful. I'm wondering if we should convert the full headers object to a dict and return them all. Are we sure we only need the extended headers? The entire headers object may not be necessary, but I don't think it would hurt to return them all either.

Also, when ready to merge, don't forget to add your signed off line. Thanks.

For now I just went with all custom headers, but as you mentioned including all headers in the result won't hurt either. I'll adjust the change and also adapt the PR within your guidelines. Sorry about that.

Arksine commented 2 months ago

Thanks.