Opened 7 years ago

Closed 7 years ago

#2060 closed task (complete)

ZoneFinder::Context::getAtOrigin

Reported by: jinmei Owned by: jinmei
Priority: medium Milestone: Sprint-20121009
Component: data source Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 5 Add Hours to Ticket: 0
Total Hours: 4.55 Internal?: no

Description

See #2056 for background. This is another one of such things,
referring to trac1607exp branch:

    // Called to get origin NS or SOA (although the method is generalized).
    // Default version: internally calls find() for the origin name and the
    //   given type, and return the result.
    // Optimized in-memory version: it has a shortcut to the origin node.
    //   get directly the requested RRset there and return it.

And in this ticket we'll only need to do the "default version". It
should be essentially straightforward refactoring.

Subtickets

Change History (11)

comment:1 Changed 7 years ago by jinmei

  • Milestone set to Next-Sprint-Proposed

comment:2 Changed 7 years ago by muks

  • Estimated Difficulty changed from 0 to 5

comment:3 Changed 7 years ago by muks

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

comment:4 Changed 7 years ago by jinmei

  • Owner set to jinmei
  • Status changed from new to accepted

comment:5 follow-up: Changed 7 years ago by jinmei

trac2060 is ready for review.

Actually the only related change is the first commit (d73842e).

But since it's pretty small I wanted to piggy back one outstanding
issue described in #1767. It's the second commit (e0ce6d2).
The diff for this commit is a bit large, but what it does is
essentially simple: introduce a generic derived class of
ZoneFinder::Context, reducing the overhead of constructing the base
class, and have most of the others (except in-memory) use the generic
one. I decided to not to make the base class fully abstract (it still
has member variables and non pure virtual methods), considering the
balance between design purity and convenience.

I hope the additional review load for the second commit is acceptable,
but if not, I'm okay with deferring it to #1767.

No changelog is planned for this ticket.

comment:6 Changed 7 years ago by jinmei

  • Owner changed from jinmei to UnAssigned
  • Status changed from accepted to reviewing

comment:7 Changed 7 years ago by vorner

  • Owner changed from UnAssigned to vorner

comment:8 in reply to: ↑ 5 ; follow-up: Changed 7 years ago by vorner

  • Owner changed from vorner to jinmei
  • Total Hours changed from 0 to 0.55

Hello

Replying to jinmei:

Actually the only related change is the first commit (d73842e).

Which ticket is it that'll actually use this new method? I think we could
switch to it before writing the specialized versions.

I hope the additional review load for the second commit is acceptable,
but if not, I'm okay with deferring it to #1767.

That's OK. But I think the getFinder and getAllRRsets comments need to
clarify the ownership of the pointer (that it is preserved in the context, not
passed onto the caller).

Other than that, it looks OK, so after documenting it, please merge.

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

Thanks for the review.

Replying to vorner:

Hello

Replying to jinmei:

Actually the only related change is the first commit (d73842e).

Which ticket is it that'll actually use this new method? I think we could
switch to it before writing the specialized versions.

It's expected to be done in #2284.

I hope the additional review load for the second commit is acceptable,
but if not, I'm okay with deferring it to #1767.

That's OK. But I think the getFinder and getAllRRsets comments need to
clarify the ownership of the pointer (that it is preserved in the context, not
passed onto the caller).

Other than that, it looks OK, so after documenting it, please merge.

Okay, I've updated the doxygen comments. Hoping the new text is okay,
I'm going to merge the branch and close the ticket. If you don't
think the doc isn't good or something different from what you intended
please raise it. If it's minor we'll correct it on master; otherwise
we can create a new followup ticket.

comment:10 Changed 7 years ago by jinmei

merge done, closing.

comment:11 Changed 7 years ago by jinmei

  • Resolution set to complete
  • Status changed from reviewing to closed
  • Total Hours changed from 0.55 to 4.55
Note: See TracTickets for help on using tickets.