Code review comment for lp://staging/~dpb/charms/precise/haproxy/fix-service-entries

Revision history for this message
Sidnei da Silva (sidnei) wrote :

Hi David, thanks for the branch. A few comments below.

43 + service = {}
44 + # First come, first serve, make sure all options other than
45 + # "servers" are represented in the combined dict
46 + for key in old_service.keys():
47 + service[key] = old_service[key]

This can be rewritten as:

  service = old_service.copy()

56 + service["servers"] = old_servers + new_servers

This doesn't account for duplicate server entries. I would deduplicate on (ip, port). The trick is if options differ for two definitions of the same (ip, port) combination, what to do so it's deterministic. Maybe sort on the whole tuple contents and pick first?

review: Needs Fixing

« Back to merge proposal