Skip to content
This repository was archived by the owner on Apr 4, 2022. It is now read-only.

Removes EtcdLifecycle in favor of AutoRegistration and ServiceRegistry APIs#43

Open
venilnoronha wants to merge 2 commits intospring-attic:mainfrom
venilnoronha:gh-36
Open

Removes EtcdLifecycle in favor of AutoRegistration and ServiceRegistry APIs#43
venilnoronha wants to merge 2 commits intospring-attic:mainfrom
venilnoronha:gh-36

Conversation

@venilnoronha
Copy link
Contributor

Hi,

This PR removes the existing EtcdLifecycle while adding an implementation based on the AutoRegistration and ServiceRegistry APIs. See #36 for more info.

Please review and pull.

Thanks,
Venil

org.springframework.cloud.etcd.discovery.RibbonEtcdAutoConfiguration

# Discovery Client Configuration
org.springframework.cloud.client.discovery.EnableDiscoveryClient=\
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, gonna need this still.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed this.

@venilnoronha
Copy link
Contributor Author

Does it look okay now?

public EtcdLifecycle etcdLifecycle() {
return new EtcdLifecycle(client, etcdDiscoveryProperties());
@ConditionalOnMissingBean
public TtlScheduler ttlScheduler(EtcdDiscoveryProperties properties) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you explain why this is needed? Etcd has a ttl built in.

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 thought we would want to mark the service's availability via a heartbeat. What's the alternative?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You're right, How about renaming it using Heartbeat.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 601d7b8.

qiyi added a commit to qiyi/spring-cloud-etcd that referenced this pull request Mar 3, 2018
….0.RELEASE and Spring Cloud 2.0.0.x, spring-attic#46

1. Change the version to 2.0.0.BUILD-SNAPSHOT
2. Merge the spring-attic#43 from @venilnoronha for the EtcdLifecycle can't work now
3. Upgrade the etcd version to v3.3.1 for ci
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments