Skip to content
This repository has been archived by the owner. It is now read-only.

Workaround when FQDN not defined #6

Merged
merged 6 commits into from Feb 7, 2012
Merged

Workaround when FQDN not defined #6

merged 6 commits into from Feb 7, 2012

Conversation

@organicveggie
Copy link
Contributor

@organicveggie organicveggie commented Feb 1, 2012

Hi,

We've found a few circumstances in our 0.9.14 Chef environment out at EC2 where node[:fqdn] is not defined. We've never managed to get to the bottom of the problem, but it causes pychef to crash. These commits introduce a method for checking if an attribute exists and provide the ability to specify a function for the hostname_attr to provide a workaround for scenarios like using the EC2 internal host name when the fqdn attribute is not defined.

-Sean

@organicveggie
Copy link
Contributor Author

@organicveggie organicveggie commented Feb 6, 2012

Sorry about the blanket except, obviously my Python is more than a bit rusty. :)

@organicveggie
Copy link
Contributor Author

@organicveggie organicveggie commented Feb 6, 2012

Regarding the hostname attr changes... I can make the parameter a function. Or I can make the code support either a function or a string. Any preferences between the two approaches?

@organicveggie
Copy link
Contributor Author

@organicveggie organicveggie commented Feb 6, 2012

One more follow up question. As a refresher, I've got a situation where at least one of my Chef clients does not have a "fqdn" attribute. I've spent a lot of time trying to track it down, but I've never caught the guilty party. Nor do I understand how it even happened. That said, if the "fqdn" attribute exists, I would prefer to use that. If the "fqdn" attribute doesn't exist, I'm willing to fall back on the EC2 hostname.

So, it wouldn't really help me to make the hostname_attr parameter a function unless that function took something like the row.object.attributes object as parameter. Does that make sense? Any suggestions for a better approach?

@organicveggie
Copy link
Contributor Author

@organicveggie organicveggie commented Feb 6, 2012

Last comment, I swear. This could be done via a partial function:

def hostname_attr_by_string(attribute_name, node_attributes):
    return node_attributes.get_dotted(attribute_name)

class Roledef(object):
    def __init__(self, name, api, hostname_attr):
        self.hostname_attr = hasattr(hostname_attr, '__call__') ? hostname_attr : partial(hostname_attr_by_string, hostname_attr)

    def __call__(self):
        for row in Search('node', 'roles:'+self.name, api=self.api):
            yield self.hostname_attr(row.object.attributes)

Would something like that be acceptable?

@coderanger
Copy link
Owner

@coderanger coderanger commented Feb 7, 2012

I wouldn't do it quite like that, more like:

attr = self.hostname_attr(node) if callable(self.hostname_attr) else self.hostname_attr

Could also be fixed with some subclass hooks, but thats a little harder to deal with code reuse given the factory function.

@organicveggie
Copy link
Contributor Author

@organicveggie organicveggie commented Feb 7, 2012

Thanks, that's an excellent suggestion. I've taken that approach and included it in the pull request.

def use_ec2_hostname(node):
if node.attributes.has_dotted('fqdn'):
return 'fqdn'
else

This comment has been minimized.

@coderanger

coderanger Feb 7, 2012
Owner

Syntax error here :-)

@coderanger
Copy link
Owner

@coderanger coderanger commented Feb 7, 2012

Other than ^^ looks great!

@organicveggie
Copy link
Contributor Author

@organicveggie organicveggie commented Feb 7, 2012

Bah! :)

Clearly I'm spending too much time in Java/Ruby land.

coderanger added a commit that referenced this pull request Feb 7, 2012
Allow passing a method as the hostname_attr in the fabric roledefs system, this allows more complex logic to select what attribute to use for the hostname.

Also fixes line endings on some Sphinx files that were generated on Windows.
@coderanger coderanger merged commit 440acac into coderanger:master Feb 7, 2012
@coderanger
Copy link
Owner

@coderanger coderanger commented Feb 7, 2012

Thanks for the patch!

@coderanger

This comment has been minimized.

Copy link
Owner

@coderanger coderanger commented on e517f0b Mar 23, 2012

Just as a warning for when you update to the latest version, I changed this behavior in a backwards-incompatible way. On the plus side, the defaults for chef_roledefs() now do exactly what your example is :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants