Adding AWS metadata #33
Conversation
| @@ -69,6 +69,38 @@ var appInfo = client.getInstancesByAppId('YOURSERVICE'); | |||
| var appInfo = client.getInstancesByVipAddress('YOURSERVICEVIP'); | |||
| ``` | |||
|
|
|||
| ## Configuring for AWS environments | |||
|
|
|||
| For AWS environments (`dataCenterInfo.name` == `Amazon`) the client has built-in logic to request the AWS metadata that the Eureka | |||
There was a problem hiding this comment.
You probably should put the == in the code block.
| /* | ||
| Utility class for pulling AWS metadata that Eureka requires when registering as an Amazon instance (datacenter). | ||
| */ | ||
| export class AwsMetadata { |
There was a problem hiding this comment.
Should be called AWSMetadata?
There was a problem hiding this comment.
Yeah I went back and forth on that...
https://www.approxion.com/?p=303
http://stackoverflow.com/questions/15526107/acronyms-in-camelcase
I could go either way. I'll let @daveedgarcia chime in. :)
There was a problem hiding this comment.
Yeah it's really tricky. I generally think that acronyms should be caps (especially when it's always stylized that way), but I'm fine if we think otherwise.
There was a problem hiding this comment.
Yeah I think it gets weird when you have more than one acronym in a single instance.
HTTPURLConnection
HttpUrlConnection
Etc.
There was a problem hiding this comment.
Yeah agreed. Let's just standardize on all caps + snake. HTTP_URL_Connection. 
There was a problem hiding this comment.
Since usage of this class is encapsulated in the client... I'm going to say this doesn't matter anyway.
There was a problem hiding this comment.
With @kesne on this one but yeah doesn't matter.
|
LGTM other than one comment on naming. |
See #32 for more information.