Opened 9 years ago

Closed 9 years ago

#247 closed defect (fixed)

map config data can't be reconfigured by bindctl

Reported by: zzchen_pku Owned by: jelte
Priority: medium Milestone:
Component: configuration Version:
Keywords: map Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: Add Hours to Ticket:
Total Hours: Internal?: no

Description

Map config data remains the same after committing changes. Config data of other types can be reconfigured successfully.

Subtickets

Attachments (1)

bind10_flatten_nested_dicts.patch (3.5 KB) - added by jelte 9 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 follow-up: Changed 9 years ago by jelte

  • billable set to 1
  • Internal? unset

Hmm, seems to work fine for me, have you got a specific case that fails?

I do notice some other problems, in bindctl map values aren't printed nicely (if at all), and the specfile verifier insists on a default value of the map (but i think that's a more general one)

comment:2 Changed 9 years ago by zhanglikun

I find the map type content haven't been checked now. should we do it now? or it has some relationship with this ticket?

    elif data_type == "map" and type(value) != dict:
        # todo: check types of map contents too
        raise isc.cc.data.DataTypeError(str(value) + " is not a map")

comment:3 in reply to: ↑ 1 Changed 9 years ago by zzchen_pku

Replying to jelte:

I do notice some other problems, in bindctl map values aren't printed nicely (if at all),

Below is my config data and operate steps:
config data:
...

{

"item_name": "File_Channel",
"item_type": "map",
"item_optional": false,
"item_default":{

"log_file": "@@LOCALSTATEDIR@@/@PACKAGE@/zone.sqlite3",
"log_versions" : 5

},

"map_item_spec":[
{

"item_name": "log_file",
"item_type": "string",
"item_optional": false,
"item_default": "/root/spec"

},
{

"item_name": "log_version",
"item_type": "integer",
"item_optional": false,
"item_default": 5

}

]

},

...


operate steps:

config go Xfrout

/Xfrout> config show File_Channel/log_file
log_file: /root/spec string
/Xfrout> config show File_Channel/log_version
log_version: 5 integer
/Xfrout> config set File_Channel/log_version 6
/Xfrout> config show File_Channel/log_version
log_version: 6 integer (modified)
/Xfrout> config commit
/Xfrout> config show File_Channel/log_version
log_version: 5 integer

comment:4 Changed 9 years ago by zhanglikun

Jerry, this is because you didn't save the config data in xfrout's config_handler().

comment:5 follow-up: Changed 9 years ago by jelte

Actually, that is not by definition necessary; if config options are read from the ccsession directly when used, they should update automatically (of course if you copy your settings for more efficient access, then you do need to copy them on updates).

But I don't think that's the problem here; If I simply add the above snippet to the spec file, xfrout answers to the update command with an error ("unknown config data", due to the check in xfrout.py.in line 432+).

If the module answers with an error, the configuration manager rolls back the change, so that might be why it's not stored.

Perhaps I should add some debug logging to cfgmgr for this :)

comment:6 in reply to: ↑ 5 Changed 9 years ago by zzchen_pku

Replying to jelte:

But I don't think that's the problem here; If I simply add the above snippet to the spec file, xfrout answers to the update command with an error ("unknown config data", due to the check in xfrout.py.in line 432+).

Yeah.
I print out the keys of config data.

def __init__(self):
...
self._config_data = self._cc.get_full_config()
for key in self._config_data:
    print (str(key))
...

The two keys of map config data are: "File_Channel/log_file" and "File_Channel/log_version".
When we update "File_Channel/log_version" by bindctl using:
$config set File_Channel/log_version 6
$config commit

 def config_handler(self, new_config):
...
     answer = create_answer(0)
     for key in new_config:
         print (str(key))
...

The key name is "File_Channel", so it fail to find the key in self._config_data, xfrout answers to the update command with an error.

comment:7 Changed 9 years ago by jelte

Ah, I see. The command does not have the keys "File_Channel/log_file" and "File_Channel/log_version" in its dict, but a new dict named by the key "File_Channel".

Which does not match the output gotten from get_full_config earlier. get_full_config was not designed for this purpose (which points to a lack of documentation there). But we could certainly add something that would return it as a nested dict.

Or we could do it the other way around and provide a helper function that 'flattens' a dict (because with the first option, you'd have to recursively go through the data in the check routine that returns the error).

Any preference?

Changed 9 years ago by jelte

comment:8 Changed 9 years ago by jelte

Attached a patch with one way to get around the problem (the second solution from my previous comment), i've added a function that 'flattens' dicts, so that you can go over them in a simple for loop.

let me know what you think :)

comment:9 Changed 9 years ago by zzchen_pku

I think the second one is better.
The patch works fine for me:)

comment:10 Changed 9 years ago by zzchen_pku

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.