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

Rename and remove deprecated functions in the cluster API #38

Merged
merged 3 commits into from
Sep 3, 2024

Conversation

bjosv
Copy link
Collaborator

@bjosv bjosv commented Jun 29, 2024

Rename API functions to get a standalone context from a cluster node, which are needed to support PUB/SUB.

ctx_get_by_node  -> valkeyClusterGetValkeyContext
actx_get_by_node -> valkeyClusterGetValkeyAsyncContext
  • Remove internal parse_cluster_* functions from the cluster API. Should not be needed by users.
  • Remove deprecated functions in the cluster API.

@bjosv bjosv force-pushed the remove-deprecated-functions branch from 7d80a67 to cf9c329 Compare June 29, 2024 12:35
@bjosv bjosv force-pushed the remove-deprecated-functions branch from cf9c329 to 9d81e46 Compare September 2, 2024 08:22
These APIs are needed to support PUB/SUB, rename them for
public consumption:

  ctx_get_by_node  -> valkeyClusterGetValkeyContext
  actx_get_by_node -> valkeyClusterGetValkeyAsyncContext

Signed-off-by: Björn Svensson <[email protected]>
@bjosv bjosv changed the title Remove deprecated functions in cluster API Rename and remove deprecated functions in the cluster API Sep 2, 2024
Copy link
Collaborator

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

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

Looks good!

Do you want to update the docs markdown file in the same PR?

We should remember this change for the migration guide. We could create a label for the PR for that.

@bjosv
Copy link
Collaborator Author

bjosv commented Sep 3, 2024

The current docs don't mention xx_get_by_node as these APIs shouldn't be needed by the common user.
We should add it to the migration guide though, which I'm working on. The labels might be a good start and the release-drafter needs labels too. I'll look at it.

@bjosv bjosv added the breakingchange Indicates a possible backwards incompatible change label Sep 3, 2024
@zuiderkwast
Copy link
Collaborator

How is this a breaking change? We haven't released anything yet.

@bjosv bjosv removed the breakingchange Indicates a possible backwards incompatible change label Sep 3, 2024
@bjosv
Copy link
Collaborator Author

bjosv commented Sep 3, 2024

Remove label, used it when constructing #85

@bjosv bjosv merged commit 207b95c into valkey-io:main Sep 3, 2024
41 checks passed
@bjosv bjosv deleted the remove-deprecated-functions branch September 3, 2024 11:48
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