Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.
/ pulp Public archive

1004346 - deal with bindings w/ binding_config = None #620

Merged
merged 2 commits into from
Sep 18, 2013
Merged

Conversation

jortel
Copy link
Contributor

@jortel jortel commented Sep 17, 2013

https://bugzilla.redhat.com/show_bug.cgi?id=1004346

The problem is that non-nodes bindings are being created with (None) value for binding_config. IMHO, a configuration attribute should be defaulted to {} rather than None.

strategy = b['binding_config'].get('strategy', constants.DEFAULT_STRATEGY)
cfg = b['binding_config']
if not isinstance(cfg, dict):
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

If the config is not a dict, do we want to log an error?

@jlconnor
Copy link
Contributor

Just wondering if we want some reporting if you get a config you're not expecting. Otherwise looks good. Merge on at will

a binding_config of None.
"""
collection = Bind.get_collection()
collection.update(QUERY, UPDATE, multi=True, safe=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, no looping, so I love it 🎢

@jlconnor
Copy link
Contributor

Looks good, merge on sir!

jortel added a commit that referenced this pull request Sep 18, 2013
1004346 - deal with bindings w/ binding_config = None
@jortel jortel merged commit 5523972 into master Sep 18, 2013
@jortel jortel deleted the jortel-1004346 branch September 18, 2013 20:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants