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.

Sign up for GitHub

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

Jump to bottom

Added the UniqueIdentifier interface #85

Merged
merged 1 commit into from
Feb 19, 2014
Merged

Added the UniqueIdentifier interface #85

merged 1 commit into from
Feb 19, 2014

Conversation

rthomas
Copy link

@rthomas rthomas commented Feb 17, 2014

Added the UniqueIdentifier interface and modified InstanceInfo to call this getId method if this interface is implemented, rather than checking if name is Name.AMAZON. Updated AmazonInfo to use this.

This change is to allow non-amazon instances to be identified by a mechanism other than their hostname - e.g. multiple docker containers on the same host, registering with Eureka with the docker-host address.

Added the UniqueIdentifier interface and modified InstanceInfo to cal…
3d2a2e5 
…l this getId method if this interface is implemented, rather than checking if name is Name.AMAZON. Updated AmazonInfo to use this.

This change is to allow non-amazon instances to be identified by a mechanism other than their hostname - e.g. multiple docker containers on the same host, registering with Eureka with the docker-host address.
@rthomas
Copy link
Author

rthomas commented Feb 17, 2014

Clone of PR #84, which was closed due to additional commits.

@cloudbees-pull-request-builder
Copy link

eureka-pull-requests #55 SUCCESS
This pull request looks good

@NiteshKant
Copy link
Contributor

Thanks Ryan for the contribution!

Since, you are only using UniqueIdentifier for DataCenterInfo & there isn't any general purpose use of the interface, do you think it is simple to just add the method getInstanceId() in DataCenterInfo interface itself?

@rthomas
Copy link
Author

rthomas commented Feb 17, 2014

The reason I chose to have a new interface was to not introduce a breaking change for existing implementations of DataCenterInfo.

If that is acceptable however, I am happy to move it across.

Cheers,

ryan

@NiteshKant
Copy link
Contributor

That makes sense, I will merge the pull request.

On Monday, February 17, 2014, Ryan Thomas [email protected] wrote:

The reason I chose to have a new interface was to not introduce a breaking
change for existing implementations of DataCenterInfo.

If that is acceptable however, I am happy to move it across.

Cheers,

ryan

Reply to this email directly or view it on GitHubhttps://github.com//pull/85#issuecomment-35321350
.

NiteshKant added a commit that referenced this pull request Feb 19, 2014
@NiteshKant
Merge pull request #85 from atlassian/unique-identifier-pr
4ca5d76 
Added the UniqueIdentifier interface
@NiteshKant NiteshKant merged commit 4ca5d76 into Netflix:master Feb 19, 2014
@NiteshKant NiteshKant mentioned this pull request Feb 21, 2014
Closed
@abaculus abaculus mentioned this pull request Nov 12, 2014
Closed
@abaculus abaculus mentioned this pull request Dec 9, 2014
Closed
st3v added a commit to st3v/fargo that referenced this pull request Sep 5, 2015
@st3v
Allow for customizable instance IDs
fdd2ee4 
For context:
Netflix/eureka#85
Netflix/eureka#24
@st3v st3v mentioned this pull request Sep 6, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rthomas @cloudbees-pull-request-builder @NiteshKant