Opened 7 years ago

Closed 7 years ago

#2184 closed defect (fixed)

adding and removing items to/from lists/dicts for item_type=='any'

Reported by: jelte Owned by: jelte
Priority: medium Milestone: Sprint-20120821
Component: ~bind-ctl (obsolete) Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 3 Add Hours to Ticket: 0
Total Hours: 0.96 Internal?: no

Description

As noted in the current documentation ticket #2133, when we have a spec containing an item of type 'any', bindctl has a hard time modifying the data, and can only set it as a whole.

This will obviously not do for lists and named sets (as used in for instance the 'MasterFiles?' config).

We discussed this, and ideally we'd have some form of dynamic overlay here, determined at runtime by the configuration manager, and perhaps data types even need to be context-sensitive (where the format of an item depends on the value of another item). This is far from easy to implement, and given that the current implementation is already creaking at the seams, I propose we take this into account in the rework of it (as proposed earlier, although that proposal did not take this problem into account yet).

There is, however, something we can do reasonably fast, that would make it at least a bit usable; we can have bindctl determine the format based on the current value;

  • if it is a primitive type, it can only be set directly
  • if it is a list, you can add/remove like normal lists
  • if it is a dict, you can add/remove like with named sets (it can by definition not be a map, since that would've been in the spec)

Obviously this would still not be ideal; you still have to manually set a value, but after that, you can add/remove normally.

I already have some code, and I propose we resolve this quickly, and ideally even before the release that adds the new datasource config changes.

Subtickets

Change History (8)

comment:1 Changed 7 years ago by jelte

  • Estimated Difficulty changed from 0 to 3
  • Milestone changed from Next-Sprint-Proposed to Sprint-20120821

comment:2 Changed 7 years ago by jelte

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

comment:3 Changed 7 years ago by jelte

  • Owner changed from jelte to UnAssigned
  • Status changed from assigned to reviewing

Okay, this is ready for review.

I sneaked in another fix, dunno if you've seen the error
'Error: data_sources/classes/IN[X]/params not found' at some point, but it was a case where bindctl did not see the difference between no value and a none value.

Also updated the guide part about having to set the params value in one command.

comment:4 Changed 7 years ago by vorner

  • Owner changed from UnAssigned to vorner

comment:5 Changed 7 years ago by vorner

  • Owner changed from vorner to jelte

Hello

Looking at the code, it seems it's OK and blends well into the surrounding code.

However, I noticed few things that may be some debugging leftovers:

-                item_name =  isc.cc.data.parse_value_str(value_str)
+                #item_name =  isc.cc.data.parse_value_str(value_str)
+                item_name = value_str
                 value, status = self.get_value(identifier)
-                if status == self.NONE and not spec_part['item_optional']:
+                if status == self.NONE and not spec_part['item_optional']:# and\
+                   #not ('item_default' in spec_part):
                     raise isc.cc.data.DataNotFoundError(identifier + " not found")

This one, what does it print? When I'm guessing what should go out, I think the sentence might need some verb or so somewhere.

raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list or a named_set " + str(module_spec) + " and type " + str(type(cur_value)))

I think these are small things, so if you can have a look at them before tomorrow morning, I think I can still review them before I leave.

comment:6 Changed 7 years ago by jelte

  • Owner changed from jelte to vorner

Actually, that third one should not need to print the python base type and the module spec in the first place, just saying 'not a list or a named_set' should be enough, so I remove the last part.

Also removed the dead code from the first two things (not so much debug as minor refactor leftovers)

comment:7 Changed 7 years ago by vorner

  • Owner changed from vorner to jelte
  • Total Hours changed from 0 to 0.96

Thank you, I think it can be merged.

comment:8 Changed 7 years ago by jelte

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

Thanks, merged, closing ticket

Note: See TracTickets for help on using tickets.