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

The hostname should be fullyQualifiedDomainName instead of hostname #280

Closed
wants to merge 2 commits into
base: trunk
from

Conversation

Projects
None yet
3 participants
@RoelVanNyen
Contributor

RoelVanNyen commented Apr 22, 2014

Otherwise we don`t know which machine this really is, you only see a part of the hostname.

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Apr 23, 2014

Member

@RoelVanNyen Can you please update the failing tests?

Thanks

Member

Kami commented Apr 23, 2014

@RoelVanNyen Can you please update the failing tests?

Thanks

@RoelVanNyen

This comment has been minimized.

Show comment
Hide comment
@RoelVanNyen

RoelVanNyen Apr 23, 2014

Contributor

Should be fixed now.

Thanks

Contributor

RoelVanNyen commented Apr 23, 2014

Should be fixed now.

Thanks

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Apr 23, 2014

Member

@RoelVanNyen Actually, sorry. I just quickly glanced over this the first time.

I think a better approach would be to store this value in the extra['hostname'] attribute.

Member

Kami commented Apr 23, 2014

@RoelVanNyen Actually, sorry. I just quickly glanced over this the first time.

I think a better approach would be to store this value in the extra['hostname'] attribute.

@RoelVanNyen

This comment has been minimized.

Show comment
Hide comment
@RoelVanNyen

RoelVanNyen Apr 23, 2014

Contributor

If you go to the softlayer panel and check the hosts you will see the names are equal to the fullyQualifiedDomainName and not the hostname, so it would be logical for me to use host['fullyQualifiedDomainName'] instead. Howhever if you really don`t want to do this then I would suggest putting the field into extra['fullyQualifiedDomainName'] instead of extra['hostname'].

What do you think?
Thanks

Contributor

RoelVanNyen commented Apr 23, 2014

If you go to the softlayer panel and check the hosts you will see the names are equal to the fullyQualifiedDomainName and not the hostname, so it would be logical for me to use host['fullyQualifiedDomainName'] instead. Howhever if you really don`t want to do this then I would suggest putting the field into extra['fullyQualifiedDomainName'] instead of extra['hostname'].

What do you think?
Thanks

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Apr 23, 2014

Member

@RoelVanNyen Ah, if the Softlayer control panel uses fullyQualifiedDomainName for the display name, then using that in Libcloud makes sense as well.

Member

Kami commented Apr 23, 2014

@RoelVanNyen Ah, if the Softlayer control panel uses fullyQualifiedDomainName for the display name, then using that in Libcloud makes sense as well.

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Apr 23, 2014

Member

Commits squashed and merged into trunk. Thanks.

Member

Kami commented Apr 23, 2014

Commits squashed and merged into trunk. Thanks.

@RoelVanNyen

This comment has been minimized.

Show comment
Hide comment
@RoelVanNyen

RoelVanNyen Apr 23, 2014

Contributor

Cool. However I don`t see your commit anywhere.
Where is it committed ?

Cheers,

Contributor

RoelVanNyen commented Apr 23, 2014

Cool. However I don`t see your commit anywhere.
Where is it committed ?

Cheers,

@asfgit asfgit closed this in 48d3bfd Apr 23, 2014

@phamnamlong

This comment has been minimized.

Show comment
Hide comment
@phamnamlong

phamnamlong Sep 9, 2014

Hi @RoelVanNyen, I find this fix quite problematic. Imagine the following scenario:

You create a node by:

host = conn.create_node(name="first", image=image, size=size) 

Then, later you want to find this node by name:

def found_with_id(id):
  nodes = conn.list_nodes();
    for node in nodes:
      if node.name == id: # this is where the originally created name is not what it is now
        return node
  return None

Doesn't it break the generic purpose of libcloud where you will certainly have to "if conn is the SoftLayer" then check node.name == id + "example.com".

I suggest that the name is still the original created name while you will add extra attribute fullyQualifiedDomainName instead.

phamnamlong commented Sep 9, 2014

Hi @RoelVanNyen, I find this fix quite problematic. Imagine the following scenario:

You create a node by:

host = conn.create_node(name="first", image=image, size=size) 

Then, later you want to find this node by name:

def found_with_id(id):
  nodes = conn.list_nodes();
    for node in nodes:
      if node.name == id: # this is where the originally created name is not what it is now
        return node
  return None

Doesn't it break the generic purpose of libcloud where you will certainly have to "if conn is the SoftLayer" then check node.name == id + "example.com".

I suggest that the name is still the original created name while you will add extra attribute fullyQualifiedDomainName instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment