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

Clean old discovery metadata aerospike-namespaces #39

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

damsallem
Copy link
Contributor

No description provided.

# The key to discover Aerospike's namespaces through service discovery
# Set it to "" to instead discover namespaces automatically via the info command
# DEPRECATED since 02/19/2024
namespace_meta_key: "aerospike-namespaces"
# The key prefix to discover Aerospike's namespaces through service discovery
# old "namespace_meta_key" has been DEPRECATED because of 512 bytes limitation of the value in consul.
Copy link

Choose a reason for hiding this comment

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

I don't think keeping this comment bring useful information (it is in commit messages)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely, patching it !

if !ready {
continue
}
ns := metaKey[len(conf.AerospikeEndpointConfig.NamespaceMetaKeyPrefix):] // MetaKey is like : "aerospike-monitoring-closeststore"
// MetaKey is like : "aerospike-monitoring-closeststore"
Copy link

Choose a reason for hiding this comment

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

I would remove reference to a real namespace, use foobar instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done !

* This metadata is no more added to consul
@damsallem damsallem merged commit c72af41 into criteo:main Feb 26, 2024
1 check passed
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