CI failed, but I think just a flaky run
Replied to the existing questions
Diff comments:
> diff --git a/lib/lp/bugs/interfaces/vulnerability.py
> b/lib/lp/bugs/interfaces/vulnerability.py
> index 90a9829..bda5c1a 100644
> --- a/lib/lp/bugs/interfaces/vulnerability.py
> +++ b/lib/lp/bugs/interfaces/vulnerability.py
> @@ -381,6 +394,94 @@ class IVulnerabilitySet(Interface):
> def findByIds(vulnerability_ids, visible_by_user=None):
> """Returns the vulnerabilities with the given IDs."""
>
> + @operation_parameters(
> + handler=Choice(
> + title=_("The handler to import or export vulnerability data"),
> + readonly=True,
> + required=True,
> + vocabulary=VulnerabilityHandlerEnum,
> + ),
> + git_repository=Reference(schema=IGitRepository),
> + git_ref=TextLine(
> + title=_("The git reference to import from."),
> + required=True,
> + ),
> + git_paths=List(
> + title=_("The list of paths to import from."),
> + value_type=TextLine(),
> + required=True,
> + ),
> + information_type=copy_field(
> + IVulnerabilityEditableAttributes["information_type"]
> + ),
> + import_since_commit_sha1=TextLine(
> + title=_(
> + "Commit sha1 from which to do the import. All files added or
> "
> + "modified since this commit will be imported."
> + ),
> + required=False,
> + ),
> + )
> + @call_with(requester=REQUEST_USER)
> + @export_write_operation()
> + @operation_for_version("devel")
> + def importData(
> + requester,
> + handler,
> + git_repository,
> + git_ref,
> + git_paths,
> + information_type,
> + import_since_commit_sha1,
> + ):
> + """Trigger a vulnerability data import
> +
> + :param handler: The handler that will perform the import.
> + :param git_repository: The git repository to import from.
> + :param git_ref: The git reference to import from.
> + :param git_paths: The paths in the git repo to import from.
Hmm, specially given that we for sure want this parameter for SOSS and UCT
(because we really only want the '/active' changes, right?), it really won't
make a huge difference for now, and I agree it makes it a lot more conscious.
Let's keep it (maybe add just a comment with `[""] will get everything`) and
we'll evolve the functionality as we go - can even be picked up by now joiners
as it should be quite simple
> + :param information_type: The information type of the vulnerability.
> + If the git repo is private, the information type must be PRIVATE.
> + :param import_since_commit_sha1: The commit SHA1 to import from.
> + If null, import all data.
> + """
> +
> + @operation_parameters(
> + handler=Choice(
> + title=_("The handler to import or export vulnerability data"),
> + readonly=True,
> + required=True,
> + vocabulary=VulnerabilityHandlerEnum,
> + ),
> + sources=List(
> + title=_(
> + "List of objects to get data from. If null, import all data."
> + ),
> + required=False,
> + ),
> + )
> + @call_with(requester=REQUEST_USER)
> + @export_write_operation()
> + @operation_for_version("devel")
> + def exportData(
> + requester,
> + handler,
> + sources,
> + ):
> + """Trigger a vulnerability data export
> +
> + :param handler: The handler that will perform the export.
> + :param sources: List of objects to get data from. If null, import all
> + data.
> + """
> +
> + @collection_default_content()
> + def empty_list():
> + """Return an empty collection of vulnerabilities.
> +
> + This only exists to keep lazr.restful happy.
> + """
> +
>
> class IVulnerabilityActivity(Interface):
> """`IVulnerabilityActivity` attributes that require launchpad.View."""
> diff --git a/lib/lp/bugs/model/vulnerability.py
> b/lib/lp/bugs/model/vulnerability.py
> index c33cc0f..4d4ba07 100644
> --- a/lib/lp/bugs/model/vulnerability.py
> +++ b/lib/lp/bugs/model/vulnerability.py
> @@ -382,6 +392,96 @@ class VulnerabilitySet:
> clauses.append(get_vulnerability_privacy_filter(visible_by_user))
> return IStore(Vulnerability).find(Vulnerability, *clauses)
>
> + def importData(
> + self,
> + requester,
> + handler,
> + git_repository,
> + git_ref,
> + git_paths,
> + information_type,
> + import_since_commit_sha1,
> + ):
> + """See `IVulnerabilitySet`."""
> +
> + if not getFeatureFlag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG):
> + raise NotImplementedError(
> + "Vulnerability import API is currently disabled"
> + )
> +
> + # Launchpad's object permissions automatically check requester's
> + # permissions to git repo
> +
> + # Check requester's permissions to handler
> + from lp.bugs.enums import VulnerabilityHandlerEnum
> + from lp.bugs.scripts.soss.sossimport import SOSSImporter
> +
> + if handler == VulnerabilityHandlerEnum.SOSS:
> + importer = SOSSImporter()
> + else:
> + raise Exception("Handler not found")
> +
> + if not importer.checkUserPermissions(requester):
> + raise UnauthorizedVulnerabilityHandler(
> + f"You don't have enough rights to use {handler}"
> + )
> +
> + # If repository is private, information_type must be PRIVATE
> + if (
> + git_repository.private
> + and information_type in PUBLIC_INFORMATION_TYPES
> + ):
> + # TODO check what's usually the validation error codes we use
Hmmmmmm, it seems like when I try to check for `validate` functions most of
them seem to raise specific errors or AssertionError. I also see a
IncompatibleArguments, maybe could also work for this one in particular
For non existing git ref in a git repo etc I'd say it should be NotFoundError
> + raise ValueError(
> + "information_type must be PRIVATE when importing from a "
> + "private git repository"
> + )
> +
> + repo_ref = git_repository.getRefByPath(git_ref)
> + if repo_ref is None:
> + raise ValueError(
> + f"{git_ref} does not exist in the specified git repository"
> + )
> +
> + # Check import_since_commit_sha1 exists in git_ref
> + if import_since_commit_sha1 and not git_repository.checkCommitInRef(
> + import_since_commit_sha1, git_ref
> + ):
> + raise ValueError(
> + f"{import_since_commit_sha1} does not exist in {git_ref}"
> + )
> +
> + # Trigger the import job after validations pass
> + job_source = getUtility(IImportVulnerabilityJobSource)
> + job_source.create(
Right, maybe the exception that it raises is already the right one.
Yeah, let's create a small ticket to return the in progress job details here,
and leave this for now!
> + handler,
> + git_repository.git_https_url,
> + git_ref,
> + git_paths,
> + information_type.value,
> + import_since_commit_sha1,
> + )
> +
> + return None
> +
> + def exportData(
> + self,
> + requester,
> + handler,
> + sources,
> + ):
> + """See `IVulnerabilitySet`."""
> +
> + raise NotImplementedError(
> + "Vulnerability export API is currently disabled"
> + )
> +
> + return None
> +
> + def empty_list(self):
> + """See `IVulnerabilitySet`."""
> + return []
> +
>
> @implementer(IVulnerabilityActivity)
> class VulnerabilityActivity(StormBase):
--
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/491584
Your team Launchpad code reviewers is requested to review the proposed merge of
~enriqueesanchz/launchpad:add-import-export-endpoints into launchpad:master.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
