Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#945 closed defect (fixed)

zonemgr crashes on some configs

Reported by: jelte Owned by: jelte
Priority: very high Milestone: Sprint-20110531
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 2.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

also, when updating values, other previously configured values can be set to None instead of kept, and when something goes wrong, some new values are kept while others are not.

Subtickets

Attachments (1)

diff (1.1 KB) - added by jinmei 8 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 8 years ago by jreed

#948 is for a related issue but not high priority.

comment:2 Changed 8 years ago by vorner

  • Owner set to jelte
  • Status changed from new to assigned

Hello.

I believe it has one problem. It is impossible to set some value to 0 (for example, if I didn't want jitter), the update would be ignored:

python3
Python 3.1.3 (r313:86834, Mar 20 2011, 12:22:30)
[GCC 4.5.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> 0 or 5
5
>>>

Because 0 is false and expression like new_config.get('reload_jitter') or self._reload_jitter will take the original in this case.
(This is the reason why perl 5.10 introduced the // operator, but I don't think python has something like this, we'll need conditional expressions here I think)

It seems OK otherwise.

comment:3 follow-up: Changed 8 years ago by vorner

Ok, I've coded the change for this. Could someone check it, please?

comment:4 in reply to: ↑ 3 Changed 8 years ago by jinmei

Replying to vorner:

Ok, I've coded the change for this. Could someone check it, please?

Looks okay with one nit. I'd move the internal function a bit upward
for readability (IMO it would be more readable). Patch attached.
But I'd leave the decision on this point to you. Feel free to merge
with or without this editorial change.

Changed 8 years ago by jinmei

comment:5 Changed 8 years ago by vorner

  • Resolution set to fixed
  • Status changed from assigned to closed

OK, merged. Thanks.

comment:6 Changed 8 years ago by shane

  • Milestone changed from New Tasks to Sprint-20110531

Seems to have been done during the current sprint.

comment:7 Changed 8 years ago by jelte

  • Estimated Difficulty changed from 0.0 to 2
Note: See TracTickets for help on using tickets.