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

Use JSON header by default for /eth/v1/beacon/deposit_snapshot #5813

Merged
merged 2 commits into from
May 20, 2024

Conversation

chong-he
Copy link
Member

@chong-he chong-he commented May 19, 2024

Most of current Lighthouse beacon API uses JSON by default when the header is not specified in querying the beacon API. The /eth/v1/beacon/deposit_snapshot uses SSZ when the header is not specified. This PR changes it to use JSON to be consistent with others.

Thanks @michaelsproul for the guidance.

@chong-he chong-he added the ready-for-review The code is ready for review label May 20, 2024
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Looks great, thanks CK!

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. backwards-incompat Backwards-incompatible API change and removed ready-for-review The code is ready for review labels May 20, 2024
@michaelsproul
Copy link
Member

Marked this as backwards-incompat so we remember to mention it in the release notes. Some software that is sending the */* accept header may be surprised to receive JSON rather than SSZ.

@michaelsproul
Copy link
Member

@Mergifyio queue

Copy link

mergify bot commented May 20, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at b5de925

mergify bot added a commit that referenced this pull request May 20, 2024
@mergify mergify bot merged commit b5de925 into sigp:unstable May 20, 2024
28 checks passed
@chong-he chong-he deleted the deposit_snapshot branch July 16, 2024 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompat Backwards-incompatible API change HTTP-API ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants