Skip to content
This repository has been archived by the owner on May 15, 2020. It is now read-only.

Commit

Permalink
Merge pull request #60 from JeroennC/master
Browse files Browse the repository at this point in the history
FIX: Create PipelineRun return 409 if already exists, with detail codes to distinguish different 409s (#TIPD-2118)
  • Loading branch information
JeroennC authored Dec 6, 2019
2 parents e9c16b7 + bf501b0 commit 6954937
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 3 deletions.
13 changes: 13 additions & 0 deletions katka/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,16 @@ class Conflict(APIException):
status_code = status.HTTP_409_CONFLICT
default_detail = _("Conflict.")
default_code = "conflict"


class AlreadyExists(Conflict):
default_detail = {"detail": "Object already exists.", "code": "already_exists"}
default_code = "already_exists"


class ParentCommitMissing(Conflict):
default_detail = {
"detail": "Pipeline Run could not be created, no pipeline run found for parent commit hash.",
"code": "parent_commit_missing",
}
default_code = "parent_commit_missing"
2 changes: 1 addition & 1 deletion katka/releases.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def _add_output(pipeline_output: dict, step_output: str) -> None:
try:
pipeline_output.update(json.loads(step_output))
except json.JSONDecodeError:
logging.warning("Invalid JSON in step output")
log.warning("Invalid JSON in step output")


def _get_current_release(pipeline):
Expand Down
11 changes: 9 additions & 2 deletions katka/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from datetime import datetime

from django.conf import settings
from django.db import IntegrityError

import pytz
from katka.constants import (
Expand All @@ -11,7 +12,7 @@
PIPELINE_STATUS_QUEUED,
STEP_FINAL_STATUSES,
)
from katka.exceptions import Conflict
from katka.exceptions import AlreadyExists, ParentCommitMissing
from katka.models import (
Application,
ApplicationMetadata,
Expand Down Expand Up @@ -134,11 +135,17 @@ def _can_create(self, application, parent_hash):

return True

def create(self, request, *args, **kwargs):
try:
return super().create(request, *args, **kwargs)
except IntegrityError:
raise AlreadyExists()

def perform_create(self, serializer):
if not self._can_create(
serializer.validated_data["application"], serializer.validated_data.get("first_parent_hash")
):
raise Conflict()
raise ParentCommitMissing()
super().perform_create(serializer)

def _ready_to_run(self, pipeline_run):
Expand Down
23 changes: 23 additions & 0 deletions tests/integration/test_scmpipelinerun_view.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from uuid import UUID

from django.db import transaction

import pytest
from katka import models
from katka.constants import PIPELINE_STATUS_FAILED
Expand Down Expand Up @@ -220,6 +222,25 @@ def test_partial_update(self, client, logged_in_user, scm_pipeline_run):
p = models.SCMPipelineRun.objects.get(pk=scm_pipeline_run.public_identifier)
assert p.steps_completed == 4

def test_create_already_exists(self, client, logged_in_user, application, scm_pipeline_run):
initial_count = models.SCMPipelineRun.objects.count()
url = f"/scm-pipeline-runs/"
# Provide a different status, should not create a new PLR
data = {
"commit_hash": scm_pipeline_run.commit_hash,
"application": application.public_identifier,
"status": "skipped",
}

# This atomic transaction context allows us to do a count query after the request
with transaction.atomic():
response = client.post(url, data=data, content_type="application/json")

assert response.status_code == 409
error = response.json()
assert error["code"] == "already_exists"
assert models.SCMPipelineRun.objects.count() == initial_count

def test_create_first_commit(self, client, logged_in_user, application, scm_pipeline_run):
initial_count = models.SCMPipelineRun.objects.count()
url = f"/scm-pipeline-runs/"
Expand Down Expand Up @@ -254,6 +275,8 @@ def test_create_parent_commit_not_found(self, client, logged_in_user, applicatio
}
response = client.post(url, data=data, content_type="application/json")
assert response.status_code == 409
error = response.json()
assert error["code"] == "parent_commit_missing"
assert models.SCMPipelineRun.objects.count() == initial_count
assert not models.SCMPipelineRun.objects.filter(commit_hash="874AE57A143AEC5156FD1444A017A32137A3E34A").exists()

Expand Down

0 comments on commit 6954937

Please sign in to comment.