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

pass profile parameters in JSON body #35

Merged
merged 1 commit into from
May 17, 2024
Merged

pass profile parameters in JSON body #35

merged 1 commit into from
May 17, 2024

Conversation

d-netto
Copy link
Member

@d-netto d-netto commented May 7, 2024

This PR switches from passing the profiling parameters in the query parameters to passing them in the JSON body.

This makes it easier to forward requests to the /debug_engine endpoint from an intermediate service, which is now able to "blindly" forward the request to the /debug_engine endpoint without parsing and reconstructing the query parameters.

@d-netto d-netto requested a review from NHDaly May 7, 2024 11:29
@nickrobinson251
Copy link
Collaborator

in the ERP

please can you update the PR description so the motivation / benefit is clear to anyone outside of RAI, since this is an open-source package?

@d-netto
Copy link
Member Author

d-netto commented May 14, 2024

Sounds good. Will update the description accordingly.

@NHDaly or @Drvi: could you take a quick look at this PR?

src/ProfileEndpoints.jl Outdated Show resolved Hide resolved
src/ProfileEndpoints.jl Outdated Show resolved Hide resolved
Copy link
Member

@NHDaly NHDaly left a comment

Choose a reason for hiding this comment

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

LGTM! :) 👏
Please do switch to JSON3 though

src/ProfileEndpoints.jl Outdated Show resolved Hide resolved
Copy link
Member

@NHDaly NHDaly left a comment

Choose a reason for hiding this comment

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

LGTM

@d-netto d-netto enabled auto-merge (squash) May 16, 2024 20:24
@d-netto d-netto merged commit 5f2a42f into main May 17, 2024
8 of 12 checks passed
@d-netto d-netto deleted the dcn-json-body branch May 17, 2024 00:44
@d-netto d-netto mentioned this pull request May 17, 2024
d-netto added a commit that referenced this pull request May 17, 2024
Update ProfileEndpoints version.

Note that #35 introduced a breaking change in the sense we're no longer passing the profiling parameters for the `/debug_engine` endpoint in the query parameters, but are now passing them in the JSON body.

Because of this I'm making this PR a major version bump.
d-netto added a commit that referenced this pull request May 17, 2024
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.

3 participants