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

1.x Client InstanceInfo default address resolution #716

Merged
merged 4 commits into from
Dec 8, 2015
Merged

1.x Client InstanceInfo default address resolution #716

merged 4 commits into from
Dec 8, 2015

Conversation

qiangdavidliu
Copy link
Contributor

Adding ability to define resolution order of the local InstanceInfo's "default" network address based on data available in AmazonInfo.

@cloudbees-pull-request-builder

NetflixOSS » eureka » eureka-pull-requests #526 FAILURE
Looks like there's a problem with this pull request

* @return either a hostname or an ipAddress
*/
@JsonIgnore
public String getDefaultAddress() {
Copy link

Choose a reason for hiding this comment

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

Why this need method is needed, if it just returns hostName?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally, it would be nice to get rid of the toplevel hostname/ipAddr fields and just have users access them from DataCenterInfo. At the toplevel, what's nice to have is a default "address" that can be either hostname or ip, as defined by the registering service itself. Unfortunately, for backwards compatibility, the hostname field is today the realistic data that's used for this. Introducing this new accessor for a cleaner client API.

@cloudbees-pull-request-builder

NetflixOSS » eureka » eureka-pull-requests #527 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

NetflixOSS » eureka » eureka-pull-requests #528 SUCCESS
This pull request looks good

@tbak
Copy link

tbak commented Dec 8, 2015

Looks good

qiangdavidliu added a commit that referenced this pull request Dec 8, 2015
1.x Client InstanceInfo default address resolution
@qiangdavidliu qiangdavidliu merged commit 1f41854 into Netflix:master Dec 8, 2015
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.

None yet

3 participants