Ticket #1788 (closed task: complete)
update auth spec to specify in-memory "filetype" with sqlite3 backend
| Reported by: | jinmei | Owned by: | jinmei |
|---|---|---|---|
| Priority: | medium | Milestone: | Sprint-20120417 |
| Component: | b10-auth | Keywords: | |
| Cc: | CVSS Scoring: | ||
| Sensitive: | no | Defect Severity: | N/A |
| Sub-Project: | DNS | Feature Depending on Ticket: | xfr for in-memory |
| Estimated Difficulty: | 3 | Add Hours to Ticket: | 0 |
| Total Hours: | 3.54 | Internal?: | no |
Description
This is a subtask of #1787.
We'll update the b10-auth spec so we can specify the "type" of
in-memory data source file. This is the current spec:
{ "item_name": "zones",
"item_type": "list",
"item_optional": false,
"item_default": [],
"list_item_spec":
{ "item_name": "list_element",
"item_type": "map",
"item_optional": true,
"item_default": { "origin": "", "file": "" },
"map_item_spec": [
{ "item_name": "origin",
"item_type": "string",
"item_optional": false,
"item_default": ""
},
{ "item_name": "file",
"item_type": "string",
"item_optional": false,
"item_default": ""
}]
}
I propose adding "filetype" item. Its optional and the default is "text".
If it's "sqlite3", the specified "file" will be assumed to be an
SQLite3 database file, and some underlying magic (which will go to a
separate task) loads the zone content via the SQLite3 data source to
in-memory.
In this task we only extend the spec file and update the configuratation
parse validation to accept it. We don't actually use it yet.
There is no dependency for this task.
Change History
comment:2 Changed 15 months ago by jelte
- Milestone changed from Year 3 Task Backlog to Sprint-20120417
comment:4 Changed 15 months ago by jinmei
- Owner changed from jinmei to UnAssigned
- Status changed from accepted to reviewing
trac1788 is ready for review.
For now I don't plan to add a changelog entry. We should add it when
#1792 is done.
comment:6 follow-up: ↓ 7 Changed 15 months ago by vorner
- Owner changed from vorner to jinmei
I have few small things:
- While this is out of the scope of this ticket, I believe we could use an „enum“ config type. It would be a nicely dressed string, but with only a list of allowed values checked even in bindctl and with tam completion. This is another place it would be useful. Would you mind if I create one?
- Should the tests also check the errors_ list is empty?
- The 699f21c00c5136c0128cfbb842c1d3218de11cfe commit might have had a better message (as we talked last week on the call). It's probably not worth fixing now, but some future ones might want to hint tests for what these are in the body of the message.
- I don't think „optional“ goes very well together with „default“. Optional means the option may or may not be present at all (therefore it would have _no_ value). The default is that the option is there all the time, but it has this value if not set. The combination of both leads to the fact the default must be hardcoded in the code, which is bad (the default is at two different places). Do you think it would be possible to make it non-optional and have a default value?
comment:7 in reply to: ↑ 6 Changed 15 months ago by jinmei
Replying to vorner:
Thanks for the review.
- While this is out of the scope of this ticket, I believe we could use an „enum“ config type. It would be a nicely dressed string, but with only a list of allowed values checked even in bindctl and with tam completion. This is another place it would be useful. Would you mind if I create one?
That seems to make sense.
- Should the tests also check the errors_ list is empty?
I don't think so, and actually I realized the test could have been
simplified. Did it.
- The 699f21c00c5136c0128cfbb842c1d3218de11cfe commit might have had a better message (as we talked last week on the call). It's probably not worth fixing now, but some future ones might want to hint tests for what these are in the body of the message.
I knew that one was blunt...I don't know if it addresses the comment,
but I've revised the commit log. (see
36ec15a2721f408c1868001f8dfa4c358f2f0ec3). Note: I used git rebase
and it resulted in conflicts with the central repo, and required
re-merge.
- I don't think „optional“ goes very well together with „default“. Optional means the option may or may not be present at all (therefore it would have _no_ value). The default is that the option is there all the time, but it has this value if not set. The combination of both leads to the fact the default must be hardcoded in the code, which is bad (the default is at two different places). Do you think it would be possible to make it non-optional and have a default value?
Hmm, you're right. I'm always confused about the meaning of
"optional" for the config system. I'd argue it's quite misleading and
should be somehow clarified, but in any event the use of optional with
default was not a good choice. I could change it to non-optional, but
chose a different solution - see 24e67649252ab7dbb1b1f7c943a995998c7f2052.
In future, I'd like to make it sure that it's ensured that the config
parser is always given syntactically validated config data (right now
it can be arbitrary element data, so it also has to do some syntax
level checks, which is a kind of duplicate). At that point we can
revise the overall parser implementation and test cases, and make
things like this to non-optional in a safer manner.
comment:9 follow-up: ↓ 10 Changed 15 months ago by vorner
- Owner changed from vorner to jinmei
- Total Hours changed from 0 to 1.04
I think it can be merged.
comment:10 in reply to: ↑ 9 Changed 15 months ago by jinmei
comment:11 Changed 15 months ago by jinmei
- Status changed from reviewing to closed
- Total Hours changed from 1.04 to 3.54
- Resolution set to complete
