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

Prysm v4 - mark old endpoints for deprecation #11997

Merged
merged 26 commits into from
Feb 23, 2023

Conversation

james-prysm
Copy link
Contributor

@james-prysm james-prysm commented Feb 15, 2023

What type of PR is this?

Other

What does this PR do? Why is it needed?

We are beginning the process of deprecation for several old endpoints in favor of new ones or in favor of changed design choices. This initial PR addresses endpoints that are either unused or superseded by a newer API.

also removing a storage folder that was unused.

Which issues(s) does this PR fix?

related to #11841

@james-prysm james-prysm added API Api related tasks Cleanup Code health! labels Feb 15, 2023
@james-prysm james-prysm self-assigned this Feb 15, 2023
@james-prysm james-prysm requested a review from a team as a code owner February 15, 2023 21:11
Copy link
Contributor

@rkapka rkapka left a comment

Choose a reason for hiding this comment

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

I like to put the DEPRECATED comment at the very top to make it as visible as it can be.

proto/eth/service/beacon_chain_service.proto Outdated Show resolved Hide resolved
proto/prysm/v1alpha1/beacon_chain.proto Outdated Show resolved Hide resolved
proto/prysm/v1alpha1/beacon_chain.proto Outdated Show resolved Hide resolved
proto/prysm/v1alpha1/beacon_chain.proto Outdated Show resolved Hide resolved
proto/prysm/v1alpha1/beacon_chain.proto Outdated Show resolved Hide resolved
proto/prysm/v1alpha1/beacon_chain.proto Outdated Show resolved Hide resolved
proto/prysm/v1alpha1/beacon_chain.proto Outdated Show resolved Hide resolved
@@ -56,6 +56,7 @@ func (e *blockIdParseError) Error() string {

// GetWeakSubjectivity computes the starting epoch of the current weak subjectivity period, and then also
// determines the best block root and state root to use for a Checkpoint Sync starting from that point.
// DEPRECATED: GetWeakSubjectivity endpoint will no longer be supported
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to set the deprecated message at the bottom to conform to linting , requires comments to start with public function name

Copy link
Member

Choose a reason for hiding this comment

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

Let's go ahead and say it is no longer supported. We won't rugpull this endpoint, but we are no longer supporting it now.

@@ -92,6 +92,7 @@ var (
// Note that this will stream information whilst syncing; this is intended, to allow for complete validator state capture
// over time. If this is not required then the client can either wait until the beacon node is synced, or filter results
// based on the epoch value in the returned validator info.
// DEPRECATED: Streaming Validator Info is nolonger supported, the /eth/v1/events Beacon API endpoint will stream events and
Copy link
Contributor

Choose a reason for hiding this comment

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

The sentence is unfinished

proto/prysm/v1alpha1/beacon_chain.proto Outdated Show resolved Hide resolved
proto/prysm/v1alpha1/validator.proto Outdated Show resolved Hide resolved
proto/prysm/v1alpha1/validator.proto Show resolved Hide resolved
proto/prysm/v1alpha1/health.proto Outdated Show resolved Hide resolved
@@ -56,6 +56,7 @@ func (e *blockIdParseError) Error() string {

// GetWeakSubjectivity computes the starting epoch of the current weak subjectivity period, and then also
// determines the best block root and state root to use for a Checkpoint Sync starting from that point.
// DEPRECATED: GetWeakSubjectivity endpoint will no longer be supported
Copy link
Member

Choose a reason for hiding this comment

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

Let's go ahead and say it is no longer supported. We won't rugpull this endpoint, but we are no longer supporting it now.

proto/eth/service/beacon_chain_service.proto Outdated Show resolved Hide resolved
proto/eth/service/beacon_chain_service.proto Outdated Show resolved Hide resolved
proto/eth/service/beacon_chain_service.proto Outdated Show resolved Hide resolved
proto/eth/v1/beacon_chain.proto Outdated Show resolved Hide resolved
proto/prysm/v1alpha1/beacon_chain.proto Outdated Show resolved Hide resolved
proto/prysm/v1alpha1/beacon_chain.proto Outdated Show resolved Hide resolved
proto/prysm/v1alpha1/debug.proto Outdated Show resolved Hide resolved
proto/prysm/v1alpha1/health.proto Outdated Show resolved Hide resolved
proto/prysm/v1alpha1/validator.proto Outdated Show resolved Hide resolved
james-prysm and others added 10 commits February 22, 2023 10:23
Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>
Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>
Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>
Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>
Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>
Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>
Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>
Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>
@prylabs-bulldozer prylabs-bulldozer bot merged commit 7c30278 into develop Feb 23, 2023
@delete-merged-branch delete-merged-branch bot deleted the deprecate-old-endpoints branch February 23, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Api related tasks Cleanup Code health!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants