Opened 8 years ago

Closed 7 years ago

#2145 closed defect (fixed)

imports in isc/__init__.py

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

Description

There are a few imports in isc/init.py:

import isc.cc
import isc.config
import isc.datasrc

This causes these modules to be imported unnecessarily, we should remove them.

I suspect it may cause a few problems to simply remove them, so this is not a completely trivial cleanup task.

Subtickets

Change History (18)

comment:1 Changed 8 years ago by shane

  • Milestone New Tasks deleted

comment:2 Changed 8 years ago by jinmei

  • Milestone set to Next-Sprint-Proposed

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

This now affects isc-sysinfo, which can be more standalone if we do this import,
so I think it's a good time to fix it.

comment:4 Changed 7 years ago by jinmei

  • Milestone set to Next-Sprint-Proposed

comment:5 Changed 7 years ago by jinmei

  • Priority changed from medium to high

comment:6 Changed 7 years ago by jinmei

  • Milestone set to Next-Sprint-Proposed

comment:7 in reply to: ↑ 3 ; follow-up: Changed 7 years ago by jinmei

Replying to jinmei:

This now affects isc-sysinfo, which can be more standalone if we do this import,
so I think it's a good time to fix it.

...and in #2438 I encountered even more annoying effects due to this.
Now strongly suggest fixing it.

I believe we can remove the workaround introduced at 003f49d. Please
confirm that and clean it up.

comment:8 Changed 7 years ago by jinmei

  • Sub-Project changed from DNS to Core

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

Replying to jinmei:

Replying to jinmei:

I believe we can remove the workaround introduced at 003f49d. Please
confirm that and clean it up.

And also for 39a3150 and 610f3ec.

comment:10 Changed 7 years ago by vorner

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

comment:11 Changed 7 years ago by jelte

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

comment:12 Changed 7 years ago by jinmei

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

comment:13 Changed 7 years ago by jinmei

trac2145 is ready for review.

changes up to 39c1d35 are the main part of the branch. This should be
pretty straightforward. The question is whether these are sufficient.
I confirmed that unit tests, system tests, and lettuce tests passed
(with an unrelated fix, see below) run_bind10 and run_bindctl worked,
installed system worked. I also ran the branch on buildbots, and
they reported lettuce and distcheck failures:
http://git.bind10.isc.org/~tester/builder/BIND10-distcheck/20130206011939-FreeBSD8-i386/logs/distcheck.out
http://git.bind10.isc.org/~tester/builder/BIND10-lettuce/20130206004924-MacOSX10.6-x86_64-GCC/logs/lettuce.out

but I couldn't reproduce these. For now, I'd get it reviewed, and if
these issues don't happen either, merge it and check the buildbot again.

Commit 0749f9e is not really an issue of this task, but a kind of a
regression due to #1901. Without this change in-source lettuce tests
don't work. Since it's only for lettuce tests, I think it's okay to
piggy back the fix on this ticket.

Commit 3a185f5 is one additional cleanup related to the ticket, which
shouldn't be controversial.

The last commit 64fb39c is an unrelated cleanup, but I thought this is
better.

I don't plan to add a changelog entry for this task. It's basically
an internal cleanup and not user-visible.

comment:14 Changed 7 years ago by jinmei

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

comment:15 Changed 7 years ago by jelte

  • Owner changed from UnAssigned to jelte

comment:16 follow-up: Changed 7 years ago by jelte

  • Owner changed from jelte to jinmei

I have pushed one trivial typo fix.

Regarding the test failures, on my (debian) system I have not seen either of these, I wonder if the first one is related to switching between master and this branch (now that bob has been renamed). But both of them *appear* unrelated to this branch.

I had already cherry-picked 0749f9e into master, so consider that approved as well ;)

The code looks OK and I think this branch can be merged.

comment:17 in reply to: ↑ 16 Changed 7 years ago by jinmei

Thanks for the review.

Replying to jelte:

I have pushed one trivial typo fix.

Thanks, it looks good.

The code looks OK and I think this branch can be merged.

Okay, merge done, closing.

comment:18 Changed 7 years ago by jinmei

  • Resolution set to fixed
  • Status changed from reviewing to closed
  • Total Hours changed from 0 to 3
Note: See TracTickets for help on using tickets.