Merge ~brad-figg/+git/kteam-tools:better-nocards into ~canonical-kernel/+git/kteam-tools:master

[Brad Figg]

The commit message says it all

[Cory Todd (corytodd)]

comments inline

review: Needs Fixing

[Brad Figg (brad-figg)]

Cory,

If you read the commit message you would see an explanation of why I moved the import. I do not have the jira module. I can not access your Jira when I run this command from outside the company. I moved it down inside the check for dryrun and self.board because I have to invoke with --nocards and it won't run if it tries to import the SRUBoard class.

[Cory Todd (corytodd)]

Brad,

Hello, nice to meet you!

Yes I did read the commit message and it makes sense. I understand these tools have not been thoroughly tested for use outside the Canonical so I appreciate your help in getting things fixed up.

My suggestion included a code snippet that supports your use case and keeps the imports all at the top. The problem is that moving imports inside classes and function calls makes it hard to know when a module is actually available.

Would you mind trying this out for me? In case that snippet got list, here it is again:

try:
    from ktl.sruboard import SRUBoard, SRUBoardError
except ModuleNotFoundError:
    print("WW: sruboard module not found, you will not be able to use Jira integration")
    SRUBoard = None
    SRUBoardError = None

The import will fail, warn the user, then move on safely.

[Brad Figg (brad-figg)]

If i've specified --no-cards on the command line and I don't have the module I'm going to get that warning/error message anyway. There is nothing wrong with moving the import into the code. It's rarely done but in some cases it makes the most sense to do. Having the imports at the top of the file is a good, general, rule of thumb.

[Jose Ogando Justo (joseogando)]

Hello Brad,

For this particular MP, we are deciding that the proper way to do it is to include the library within the try sentence.

Although there is nothing technically wrong with moving the import code, this will help us to keep maintainability, I hope you understand.

Please resubmit the MP with the requested changes. In this way we can have both clean code and the functionality required.

Thanks

Unmerged commits

8047edf... by Brad Figg on 2022-09-26

create-kernel-tasks: if --nocards is specified then don't import the
SRUBoard class.

The SRUBoard class imports the jira module and not all users of
create-kernel-tasks has that module. If no jira cards are to be created
there is no need to import SRUBoard.

Signed-off-by: Brad Figg <email address hidden>

diff --git a/stable/create-kernel-tasks b/stable/create-kernel-tasks
index c62b6db..14839ee 100755
--- a/stable/create-kernel-tasks
+++ b/stable/create-kernel-tasks
@@ -13,7 +13,6 @@ sys.path.append(os.path.realpath(
 from ktl.log            import Clog, cerror, cwarn, cinfo, cdebug
 from ktl.kernel_series  import KernelSeries
 from ktl.tracking_bug2  import TrackingBugs, TrackingBugError
-from ktl.sruboard       import SRUBoard, SRUBoardError
 
 class Task():
     def __init__(self, taskset, ks_source, ks_snap=None):
@@ -436,6 +435,7 @@ class TaskSet():
     def CreateCards(self, boardname, dryrun=False, cve=False):
         if dryrun is False and self.board is None:
             try:
+                from ktl.sruboard       import SRUBoard, SRUBoardError
                 self.board = SRUBoard(self.cycletag, cve=cve)
             except SRUBoardError as e:
                 cerror(e)