auth lmdb: full support for comments #16522
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| bool LMDBBackend::getInternal(DNSName& basename, std::string_view& key) | ||
| { | ||
| // FIXME: bit of duplication from below here, but much simpler |
Check notice
Code scanning / CodeQL
FIXME comment Note
| // whether to include disabled records in the results | ||
| bool includedisabled; | ||
| // whether we are doing comments (false=records, true=comments) | ||
| bool comments; |
There was a problem hiding this comment.
Maybe prefer putting a pointer to the *dbi to use here rather than a bool? Might not be as simple as it looks.
There was a problem hiding this comment.
then we'd have to compare dbi pointers in getInternal, because the logic is different for comments vs. records (which I am unhappy about but I haven't found a clean refactor for it yet)
| "\tUnpublish the zone key with key id KEY_ID in ZONE"}}} | ||
| }; | ||
|
|
||
| static const groupCommandDispatcher commentCommands{ |
There was a problem hiding this comment.
I thought this would be part of the rrset commands. As in rrset add-comment, rrset list-comment, rrset forget-comment, rrset why-the-hell-did-I-forget-to-put-a-comment, etc.
Pull Request Test Coverage Report for Build 19674913787Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
| { | ||
| DomainInfo info; | ||
| if (!findDomain(domain_id, info)) { | ||
| throw DBException("Domain with id '" + std::to_string(domain_id) + "not found"); |
There was a problem hiding this comment.
| throw DBException("Domain with id '" + std::to_string(domain_id) + "not found"); | |
| throw DBException("Domain with id '" + std::to_string(domain_id) + "' not found"); |
| d_lookupstate.comment.content = message.get_string(); | ||
| } | ||
| catch (protozero::exception& e) { | ||
| // very little we can do about this |
There was a problem hiding this comment.
| // very little we can do about this | |
| throw DBException("got invalid serialized comment: " + e.what()); |
or
| // very little we can do about this | |
| throw; |
and handle protozero::exception in the caller (which is probably not a good idea - better wrap in a DBException or PDNSException).
There was a problem hiding this comment.
throw DBException("got invalid serialized comment: " + e.what());
on my current database (which was written by previous versions of this branch) this would throw immediately. However, I should just clean that up, and then we can throw. Ok, agreed.
This reverts commit c796957.
3 participants
Short description
WIP. TODO:
pdnsutil comment add example.com foobar A 'hello protobuf' petre->Error: std::string keyConv(const T&) [with T = DNSName; typename std::enable_if<std::is_same<T, DNSName>::value, T>::type* <anonymous> = 0; std::string = std::__cxx11::basic_string<char>] Attempt to serialize an unset DNSNameChecklist
I have: