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

Log response body #130

Closed
wants to merge 1 commit into from
Closed

Log response body #130

wants to merge 1 commit into from

Conversation

devmotion
Copy link
Member

Sometimes it can be useful to inspect the response body before parsing it with JSON3. Log level -3000 should be sufficient such that you do not fill your logs accidentally.

DilumAluthge
DilumAluthge previously approved these changes Nov 17, 2023
@devmotion
Copy link
Member Author

I fixed the PR locally but I suggest we close it: The HTTP.jl package uses the Julia logging system, and hence by enabling debug logging globally or for the HTTP package (possibly among other packages) the response is logged. Logging in FHIRClient as well could even duplicate these logging messages.

Possibly, an improvement in FHIRClient could be to support the verbosity keyword argument in HTTP.jl and forward it to HTTP.request for adjusting the verbosity of the logging messages from HTTP: https://juliaweb.github.io/HTTP.jl/stable/reference/#HTTP.request

@DilumAluthge
Copy link
Member

Hmmm. At what log level does HTTP output the full response? Is it DebugLevel (-1000)?

I'd prefer we keep this logging in FHIRClient.jl, and only output it at a lower log level, e.g. -3000.

@devmotion
Copy link
Member Author

Hmmm. At what log level does HTTP output the full response? Is it DebugLevel (-1000)?

Yes, it uses @debug with different levels of verbosity.

@DilumAluthge DilumAluthge marked this pull request as draft November 27, 2023 02:31
@DilumAluthge
Copy link
Member

Closing in favor of #132

@DilumAluthge DilumAluthge deleted the devmotion-patch-2 branch December 4, 2023 18:39
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.

2 participants