Opened 8 years ago

Closed 8 years ago

#1333 closed task (complete)

Add python wrappers of the updates to ZoneUpdater and ZoneJournalReader

Reported by: jinmei Owned by: jinmei
Priority: medium Milestone: Sprint-20111122
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:
Total Hours: 5.97 Internal?: no

Description

This task depends on #1331 and #1332. But we might want to
complete the updater part first, so that we can update xfrin
(adding diffs from IXFR-in) sooner.

Subtickets

Change History (22)

comment:1 Changed 8 years ago by jelte

  • Add Hours to Ticket changed from 0 to 5

comment:2 Changed 8 years ago by jelte

  • Add Hours to Ticket 5 deleted
  • Estimated Difficulty changed from 0 to 5

comment:3 Changed 8 years ago by jelte

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

comment:4 Changed 8 years ago by jelte

  • Milestone changed from Sprint-20111108 to Sprint-20111122

comment:5 Changed 8 years ago by jinmei

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

comment:6 Changed 8 years ago by jinmei

trac1333u is ready for review (u=update).

I completed the updater extension part first so that we can also
start working on #1376 while we are reviewing/merging #1332.

Once we merge #1332 (or at least agree on the basic interface)
we can also start working on the reader part of this task.
I suggest we add a single changelog entry when both updater and the
reader parts are completed.

comment:7 Changed 8 years ago by jinmei

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

comment:8 Changed 8 years ago by jelte

  • Owner changed from UnAssigned to jelte

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

  • Owner changed from jelte to jinmei

This part looks good and can be merged

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

Replying to jelte:

This part looks good and can be merged

Thanks, merged this part. Will keep this ticket open (unassigned)
for the rest of the task.

comment:11 Changed 8 years ago by jinmei

  • Status changed from reviewing to accepted

comment:12 Changed 8 years ago by jinmei

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

comment:13 Changed 8 years ago by jinmei

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

comment:14 Changed 8 years ago by jinmei

trac1333r is (basically) ready for review. (r = reader)

It depends on #1332, which has some ongoing discussion, but I decided
to complete it in the hope that we might be able to complete IXFR-out
before the release. At least once the API design of #1332 is fixed
this branch can be reviewed.

The first two commits are the dependency merge and should be ignored.

I believe the implementation itself is quite straightforward.

Proposed Changelog entry:

320.?	[func]		jinmei
	Python isc.datasrc: added interfaces for difference management:
	DataSourceClient.get_updater() now has the 'journaling' parameter
	to enable storing diffs to the data source, and a new class
	ZoneJournalReader was introduced to retrieve them, which can be
	created by the new DataSourceClient.get_journal_reader() method.
	(Trac #1333, git TBD)

comment:15 Changed 8 years ago by jinmei

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

comment:16 Changed 8 years ago by vorner

  • Owner changed from UnAssigned to vorner

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

  • Owner changed from vorner to jinmei

Hello.

It looks mostly OK, I noticed only two things (hopefully not because I'm sleepy now):

  • This seems to be leftover comment from the generator which could be removed?
    // Enable this if you use s# variants with PyArg_ParseTuple(), see
    // http://docs.python.org/py3k/c-api/arg.html#strings-and-buffers
    //#define PY_SSIZE_T_CLEAN
    
  • What is the purpose of this tearDown?
        def tearDown(self):
            self.dsc = None
            self.reader = None
    

Thank you

comment:18 in reply to: ↑ 17 Changed 8 years ago by jinmei

Replying to vorner:

Thanks for the review.

It looks mostly OK, I noticed only two things (hopefully not because I'm sleepy now):

  • This seems to be leftover comment from the generator which could be removed?
    // Enable this if you use s# variants with PyArg_ParseTuple(), see
    // http://docs.python.org/py3k/c-api/arg.html#strings-and-buffers
    //#define PY_SSIZE_T_CLEAN
    

I was aware that but intentionally left it there in case we'll need it
in a future extension. But now that you've pointed out, maybe it's
less confusing to just clean it up for now. So I did it.

  • What is the purpose of this tearDown?
        def tearDown(self):
            self.dsc = None
            self.reader = None
    

I clarified this in an added comment:

    def tearDown(self):
        # Some tests leave the reader in the middle of sequence, holding
        # the lock.  Since the unittest framework keeps each test object
        # until the end of the entire tests, we need to make sure the reader
        # is released at the end of each test.  The client shouldn't do harm
        # but we clean it up, too, just in case.
        self.dsc = None
        self.reader = None

comment:19 Changed 8 years ago by jinmei

  • Owner changed from jinmei to vorner

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

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

OK, then it can be merged I think.

comment:21 in reply to: ↑ 20 Changed 8 years ago by jinmei

Replying to vorner:

OK, then it can be merged I think.

Thanks, merged, closing.

comment:22 Changed 8 years ago by jinmei

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