[Merge] ~hyask/ubuntu-dev-tools:skia/i-b-f-d_multipart into ubuntu-dev-tools:main

[Mattia Rizzolo]

Review: Needs Information

Hello Skia,

thank you for this proposed MR!

This is pretty much what I envisioned a few years back (in the MRs you found), so I'd say that this is overall very good.

Could you please come back to me on these 3 small points before merging?

Diff comments:

> diff --git a/import-bug-from-debian b/import-bug-from-debian
> index 5d0fd7f..8c3623a 100755
> --- a/import-bug-from-debian
> +++ b/import-bug-from-debian
> @@ -118,7 +118,41 @@ def main():
>          bug_num = bug.bug_num
>          subject = bug.subject
>          log = debianbts.get_bug_log(bug_num)
> -        summary = log[0]["message"].get_payload()
> +        message = log[0]["message"]
> +        attachments = []
> +        if message.is_multipart():
> +            summary = ""
> +            i = 1
> +            for part in message.walk():
> +                content_type = part.get_content_type()
> +                if content_type.startswith("multipart/"):
> +                    # we're already iterating on multipart items, let's just skip the multipart extra metadata
> +                    continue

It has been a while since I touched emails in Python, however ISTR that attached emails are sometimes represented as multipart/ pieces.  I could be wrong however.

> +                elif content_type == "application/pgp-signature":
> +                    continue
> +                elif part.is_attachment():
> +                    attachments.append((i, part))
> +                elif content_type.startswith("image/"):
> +                    # images here are not attachment, they are inline, but Launchpad can't handle that,
> +                    # so let's add them as attachments
> +                    summary += f"Message part #{i}\n"
> +                    summary += f"[inline image '{part.get_filename()}']\n\n"
> +                    attachments.append((i, part))
> +                elif content_type == "text/plain":
> +                    summary += f"Message part #{i}\n"
> +                    summary += part.get_content()
> +                else:
> +                    raise RuntimeError(
> +                        f"""Unknown message part
> +Your Debian bug is too weird to be imported in Launchpad, sorry.
> +You can fix that by patching this script in ubuntu-dev-tools.
> +Faulty message part:
> +{part}"""
> +                    )

what about text/html ?  That's a very common type that I believe you should handle

> +                i += 1
> +        else:
> +            summary = log[0]["message"].get_payload()
> +
>          target = ubuntu.getSourcePackage(name=ubupackage)
>          if target is None:
>              Logger.error(
> @@ -137,12 +171,31 @@ def main():
>          Logger.debug("Subject: %s", subject)
>          Logger.debug("Description: ")
>          Logger.debug(description)
> +        for attachment in attachments:
> +            i, attachment = attachment
> +            Logger.debug(f"Attachment #{i} ({attachment.get_filename()})")
> +            Logger.debug("Content:")
> +            if attachment.get_content_type() == "text/plain":
> +                Logger.debug(attachment.get_content())

this is going to make the terminal horrible if you are handling a big log file or something.  If you really believe dumping the attachment content is useful, at least clamp it after a sensible number of characters.

> +            else:
> +                Logger.debug("[data]")
>  
>          if options.dry_run:
>              Logger.info("Dry-Run: not creating Ubuntu bug.")
>              continue
>  
>          u_bug = launchpad.bugs.createBug(target=target, title=subject, description=description)
> +        for i, attachment in attachments:
> +            name = f"#{i}-{attachment.get_filename()}"
> +            content = attachment.get_content()
> +            if isinstance(content, str):
> +                # Launchpad only wants bytes
> +                content = content.encode()
> +            u_bug.addAttachment(
> +                filename=name,
> +                data=content,
> +                comment=f"Imported from Debian bug http://bugs.debian.org/{bug_num}",
> +            )
>          d_sp = debian.getSourcePackage(name=package)
>          if d_sp is None and options.package:
>              d_sp = debian.getSourcePackage(name=options.package)


-- 
https://code.launchpad.net/~hyask/ubuntu-dev-tools/+git/ubuntu-dev-tools/+merge/468091
Your team Ubuntu Development Team is subscribed to branch ubuntu-dev-tools:main.