Opened 9 years ago

Closed 8 years ago

#926 closed enhancement (complete)

Possibility to specify variable keys for map in configuration

Reported by: vorner Owned by: jelte
Priority: medium Milestone: Sprint-20110802
Component: configuration Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: Core Feature Depending on Ticket:
Estimated Difficulty: 7.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

Currently, when we specify a map in configuration, the names of keys must be hardcoded into the spec file, and each one can have different type of value.

But sometimes it is needed to allow any names and have a single type of value (while the current simulates something like C++ struct, this would look like map<string, something>).

It could be either new value type (eg. besides having a list and map, we could have a dict) or allowing something like name "*", which would allow any name.

Subtickets

Change History (11)

comment:1 Changed 9 years ago by shane

  • Milestone New Tasks deleted

comment:2 Changed 9 years ago by jelte

  • Milestone set to Next-Sprint-Proposed

I think we could use this for both xfrin and tsig config, so I propose this for next sprint

comment:3 Changed 9 years ago by stephen

  • Estimated Difficulty changed from 0.0 to 7

comment:4 Changed 8 years ago by stephen

  • Milestone changed from Next-Sprint-Proposed to Sprint-20110712

comment:5 Changed 8 years ago by jelte

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

comment:6 Changed 8 years ago by jelte

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

I've named the new 'type' a 'named set', because it is essentially a list that does not use indices, but names; the idea is that we will be able to configure and use things like
/some_module/zones/example.com/master 1.2.3.4
instead of
/some_module/zones[1]/master 1.2.3.4

internally, it's passed as a map, just like 'normal' maps (which have fixed names as specified in the spec file). This one has variable names, but one fixed type for its values. This has the end result of behaving more like the lists we have now than the maps we have now. But since it's indexed by name, in effect it behaves like sets where the items are addressable. So i've chosen the name 'named set' instead of 'named map' which sounds like a tautology (and plain 'map' was obviously already taken, and 'dict' or 'hash' would be even more confusing imo).

While I was adding support for this, I have also fixed another issue (about out-of-bound list adressing, #1080, but we'd need to see in that ticket whether all cases have been covered now).

The actual use of this is not visible yet, we'd need to add some setting somewhere or change some current settings, which would be a good thing for another ticket (as we may need to do config version bumping and/or compatibility then). One obvious one might be Xfrin/zones.

comment:7 Changed 8 years ago by vorner

  • Owner changed from UnAssigned to vorner

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

  • Owner changed from vorner to jelte

Hello

Few small things:

  • This seems to be a result of copy-paste, the name of exception probably means DataNotFoundError?:
    except isc.cc.data.DataAlreadyPresentError as dnfe
    
  • This looks strange (the apostrophes around some words):
    // either a 'normal' map or a 'named' set
    
  • The pass here seems extra:
                result.append(prefix[:-1])
                pass
    
  • The changes in get_default_value would probably deserve some explanation or comments. I'm lost in the code.

Generally, the config_data code looks hairy. But maybe it's because it is built from special cases for different configuration types and such, that the hairiness is in the nature of problem being solved. Anyway, even if it isn't, cleaning it up probably isn't part of this task.

With regards

comment:9 in reply to: ↑ 8 Changed 8 years ago by jelte

  • Owner changed from jelte to vorner

Replying to vorner:

Hello

Few small things:

  • This seems to be a result of copy-paste, the name of exception probably means DataNotFoundError?:
    except isc.cc.data.DataAlreadyPresentError as dnfe
    

ack, changed to 'dape'

  • This looks strange (the apostrophes around some words):
    // either a 'normal' map or a 'named' set
    

updated comment

  • The pass here seems extra:
                result.append(prefix[:-1])
                pass
    

ack, removed the pass

  • The changes in get_default_value would probably deserve some explanation or comments. I'm lost in the code.

I've added a bit of rationale inline, but:

Generally, the config_data code looks hairy. But maybe it's because it is built from special cases for different configuration types and such, that the hairiness is in the nature of problem being solved. Anyway, even if it isn't, cleaning it up probably isn't part of this task.

I agree. We need to make these into nice classes and stop mucking around with raw data (especially for specification (parts))

comment:10 Changed 8 years ago by vorner

  • Owner changed from vorner to jelte

The rationale helped a little (to know what to look for in the code), I hope it looks like it should do what is says there, so let's merge it.

comment:11 Changed 8 years ago by jelte

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

Thanks, merged, closing ticket

Note: See TracTickets for help on using tickets.