Skip to content
This repository has been archived by the owner on Nov 21, 2024. It is now read-only.

Commit

Permalink
airflow: add dag integrity test, clear warnings, fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
DonHaul committed Aug 20, 2024
1 parent d67febd commit dc2b6fc
Show file tree
Hide file tree
Showing 18 changed files with 224 additions and 47 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/test-workflows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ jobs:
-v "$(pwd)"/tests:/opt/airflow/tests
-v "$(pwd)"/requirements-test.txt:/opt/airflow/requirements-test.txt
-v "$(pwd)"/data:/opt/airflow/data
-v "$(pwd)"/scripts/connections/connections.json:/opt/airflow/connections.json
-v "$(pwd)"/scripts/variables/variables.json:/opt/airflow/variables.json
-e AIRFLOW__CORE__EXECUTOR=CeleryExecutor
-e AIRFLOW__DATABASE__SQL_ALCHEMY_CONN=postgresql+psycopg2://airflow:[email protected]:5432/airflow
-e AIRFLOW__CELERY__RESULT_BACKEND=db+postgresql://airflow:[email protected]:5432/airflow
Expand All @@ -60,4 +62,4 @@ jobs:
-e AIRFLOW__CORE__LOAD_EXAMPLES="false"
-e AIRFLOW__API__AUTH_BACKENDS="airflow.api.auth.backend.basic_auth,airflow.api.auth.backend.session"
registry.cern.ch/cern-sis/inspire/workflows@${{ needs.build.outputs.image-id }}
bash -c "pip install -r requirements-test.txt && airflow db init && pytest /opt/airflow/tests"
bash -c "pip install -r requirements-test.txt && airflow db init && airflow connections import /opt/airflow/connections.json && airflow variables import /opt/airflow/variables.json && pytest /opt/airflow/tests"
79 changes: 50 additions & 29 deletions backoffice/backoffice/workflows/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from django_json_widget.widgets import JSONEditorWidget

from backoffice.management.permissions import IsAdminOrCuratorUser
from backoffice.workflows.models import Workflow
from backoffice.workflows.models import Decision, Workflow


class WorkflowsAdminSite(admin.AdminSite):
Expand All @@ -30,8 +30,41 @@ def has_permission(self, request):
)


class BaseModelAdmin(admin.ModelAdmin):
def has_view_permission(self, request, obj=None):
"""
Returns True if the user has permission to view the Workflow model.
"""
permission_check = IsAdminOrCuratorUser()
return request.user.is_superuser or permission_check.has_permission(
request, self
)

def has_change_permission(self, request, obj=None):
"""
Returns True if the user has permission to change the Workflow model.
"""
permission_check = IsAdminOrCuratorUser()
return request.user.is_superuser or permission_check.has_permission(
request, self
)

def has_delete_permission(self, request, obj=None):
"""
Returns True if the user has permission to delete the Workflow model.
"""
permission_check = IsAdminOrCuratorUser()
return request.user.is_superuser or permission_check.has_permission(
request, self
)

formfield_overrides = {
JSONField: {"widget": JSONEditorWidget},
}


@admin.register(Workflow)
class WorkflowAdmin(admin.ModelAdmin):
class WorkflowAdmin(BaseModelAdmin):
"""
Admin class for Workflow model. Define get, update and delete permissions.
"""
Expand All @@ -56,33 +89,21 @@ class WorkflowAdmin(admin.ModelAdmin):
"_updated_at",
]

formfield_overrides = {
JSONField: {"widget": JSONEditorWidget},
}

def has_view_permission(self, request, obj=None):
"""
Returns True if the user has permission to view the Workflow model.
"""
permission_check = IsAdminOrCuratorUser()
return request.user.is_superuser or permission_check.has_permission(
request, self
)
@admin.register(Decision)
class DecisionAdmin(BaseModelAdmin):
"""
Admin class for Decision model. Define get, update and delete permissions.
"""

def has_change_permission(self, request, obj=None):
"""
Returns True if the user has permission to change the Workflow model.
"""
permission_check = IsAdminOrCuratorUser()
return request.user.is_superuser or permission_check.has_permission(
request, self
)
ordering = ("-_updated_at",)
search_fields = ["id", "data"]
list_display = ("id", "action_value", "user", "workflow_id")
list_filter = [
"action",
"user",
]

def has_delete_permission(self, request, obj=None):
"""
Returns True if the user has permission to delete the Workflow model.
"""
permission_check = IsAdminOrCuratorUser()
return request.user.is_superuser or permission_check.has_permission(
request, self
)
@admin.display(description="action")
def action_value(self, obj):
return obj.action
10 changes: 9 additions & 1 deletion backoffice/backoffice/workflows/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from backoffice.workflows.constants import ResolutionDags, StatusChoices, WorkflowType
from backoffice.workflows.documents import WorkflowDocument
from backoffice.workflows.models import Workflow, WorkflowTicket
from backoffice.workflows.models import Decision, Workflow, WorkflowTicket


class WorkflowTicketSerializer(serializers.ModelSerializer):
Expand All @@ -31,6 +31,14 @@ class Meta:
fields = "__all__"


class DecisionSerializer(serializers.ModelSerializer):
workflow = serializers.PrimaryKeyRelatedField(queryset=Workflow.objects.all())

class Meta:
model = Decision
fields = "__all__"


class WorkflowDocumentSerializer(DocumentSerializer):
class Meta:
document = WorkflowDocument
Expand Down
25 changes: 23 additions & 2 deletions backoffice/backoffice/workflows/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from backoffice.workflows import airflow_utils
from backoffice.workflows.api.serializers import (
AuthorResolutionSerializer,
DecisionSerializer,
WorkflowAuthorSerializer,
WorkflowDocumentSerializer,
WorkflowSerializer,
Expand All @@ -37,11 +38,21 @@
WorkflowType,
)
from backoffice.workflows.documents import WorkflowDocument
from backoffice.workflows.models import Workflow, WorkflowTicket
from backoffice.workflows.models import Decision, Workflow, WorkflowTicket

logger = logging.getLogger(__name__)


def add_decision(workflow_id, user, action):
serializer_class = DecisionSerializer
data = {"workflow": workflow_id, "user": user, "action": action}

serializer = serializer_class(data=data)
if serializer.is_valid(raise_exception=True):
serializer.save()
return Response(serializer.data, status=status.HTTP_201_CREATED)


class WorkflowViewSet(viewsets.ModelViewSet):
queryset = Workflow.objects.all()
serializer_class = WorkflowSerializer
Expand Down Expand Up @@ -100,6 +111,15 @@ def create(self, request, *args, **kwargs):
)


class DecisionViewSet(viewsets.ModelViewSet):
queryset = Decision.objects.all()

def create(self, request, *args, **kwargs):
return add_decision(
request.data["workflow_id"], request.user, request.data["action"]
)


class AuthorWorkflowViewSet(viewsets.ViewSet):
serializer_class = WorkflowAuthorSerializer

Expand Down Expand Up @@ -160,12 +180,13 @@ def resolve(self, request, pk=None):
logger.info("Resolving data: %s", request.data)
serializer = AuthorResolutionSerializer(data=request.data)
if serializer.is_valid(raise_exception=True):
extra_data = {"create_ticket": serializer.validated_data["create_ticket"]}
extra_data = serializer.validated_data
logger.info(
"Trigger Airflow DAG: %s for %s",
ResolutionDags[serializer.validated_data["value"]],
pk,
)
add_decision(pk, request.user, serializer.validated_data["value"])

return airflow_utils.trigger_airflow_dag(
ResolutionDags[serializer.validated_data["value"]].label, pk, extra_data
Expand Down
4 changes: 4 additions & 0 deletions backoffice/backoffice/workflows/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ class WorkflowType(models.TextChoices):
class ResolutionDags(models.TextChoices):
accept = "accept", "author_create_approved_dag"
reject = "reject", "author_create_rejected_dag"
accept_curate = "accept_curate", "author_create_approved_dag"


DECISION_CHOICES = ResolutionDags.choices


class AuthorCreateDags(models.TextChoices):
Expand Down
58 changes: 58 additions & 0 deletions backoffice/backoffice/workflows/migrations/0009_decision.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Generated by Django 4.2.6 on 2024-08-15 12:25

import django.db.models.deletion
from django.conf import settings
from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
("workflows", "0008_alter_workflow_status_alter_workflow_workflow_type"),
]

operations = [
migrations.CreateModel(
name="Decision",
fields=[
(
"id",
models.BigAutoField(
auto_created=True,
primary_key=True,
serialize=False,
verbose_name="ID",
),
),
(
"action",
models.CharField(
choices=[
("accept", "author_create_approved_dag"),
("reject", "author_create_rejected_dag"),
("accept_curate", "author_create_approved_dag"),
],
max_length=30,
),
),
("_created_at", models.DateTimeField(auto_now_add=True)),
("_updated_at", models.DateTimeField(auto_now=True)),
(
"user",
models.ForeignKey(
db_column="email",
on_delete=django.db.models.deletion.CASCADE,
to=settings.AUTH_USER_MODEL,
to_field="email",
),
),
(
"workflow",
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
to="workflows.workflow",
),
),
],
),
]
13 changes: 13 additions & 0 deletions backoffice/backoffice/workflows/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

from django.db import models

from backoffice.users.models import User
from backoffice.workflows.constants import (
DECISION_CHOICES,
DEFAULT_STATUS_CHOICE,
DEFAULT_TICKET_TYPE,
DEFAULT_WORKFLOW_TYPE,
Expand Down Expand Up @@ -43,3 +45,14 @@ class WorkflowTicket(models.Model):
ticket_type = models.CharField(
max_length=30, choices=TICKET_TYPES, default=DEFAULT_TICKET_TYPE
)


class Decision(models.Model):
user = models.ForeignKey(
User, to_field="email", db_column="email", on_delete=models.CASCADE
)
workflow = models.ForeignKey(Workflow, on_delete=models.CASCADE)
action = models.CharField(max_length=30, choices=DECISION_CHOICES)

_created_at = models.DateTimeField(auto_now_add=True)
_updated_at = models.DateTimeField(auto_now=True)
37 changes: 35 additions & 2 deletions backoffice/backoffice/workflows/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from django.test import TransactionTestCase
from django.urls import reverse
from django_opensearch_dsl.registries import registry
from rest_framework import status
from rest_framework.test import APIClient

from backoffice.workflows import airflow_utils
Expand All @@ -23,6 +24,7 @@

User = get_user_model()
Workflow = apps.get_model(app_label="workflows", model_name="Workflow")
Decision = apps.get_model(app_label="workflows", model_name="Decision")


class BaseTransactionTestCase(TransactionTestCase):
Expand Down Expand Up @@ -322,7 +324,8 @@ def test_create_author(self):
@pytest.mark.vcr()
def test_accept_author(self):
self.api_client.force_authenticate(user=self.curator)
data = {"create_ticket": True, "value": "accept"}
action = "accept"
data = {"create_ticket": True, "value": action}

response = self.api_client.post(
reverse("api:workflows-authors-resolve", kwargs={"pk": self.workflow.id}),
Expand All @@ -331,6 +334,9 @@ def test_accept_author(self):
)

self.assertEqual(response.status_code, 200)
self.assertEqual(
Decision.objects.filter(workflow=self.workflow.id)[0].action, action
)

airflow_utils.delete_workflow_dag(
WORKFLOW_DAGS[WorkflowType.AUTHOR_CREATE].approve, self.workflow.id
Expand All @@ -339,7 +345,8 @@ def test_accept_author(self):
@pytest.mark.vcr()
def test_reject_author(self):
self.api_client.force_authenticate(user=self.curator)
data = {"create_ticket": True, "value": "reject"}
action = "reject"
data = {"create_ticket": True, "value": action}

response = self.api_client.post(
reverse("api:workflows-authors-resolve", kwargs={"pk": self.workflow.id}),
Expand All @@ -348,6 +355,9 @@ def test_reject_author(self):
)

self.assertEqual(response.status_code, 200)
self.assertEqual(
Decision.objects.filter(workflow=self.workflow.id)[0].action, action
)

airflow_utils.delete_workflow_dag(
WORKFLOW_DAGS[WorkflowType.AUTHOR_CREATE].reject, self.workflow.id
Expand Down Expand Up @@ -487,3 +497,26 @@ def test_ordering(self):
if previous_date is not None:
assert cur_date < previous_date
previous_date = cur_date


class TestDecisionsViewSet(BaseTransactionTestCase):
endpoint = "/api/decisions"
reset_sequences = True
fixtures = ["backoffice/fixtures/groups.json"]

def setUp(self):
super().setUp()
self.workflow = Workflow.objects.create(
data={}, status="running", core=True, is_update=False
)

def test_create_decision(self):
self.api_client.force_authenticate(user=self.curator)
data = {
"workflow_id": self.workflow.id,
"action": "accept",
}

url = reverse("api:decisions-list")
response = self.api_client.post(url, format="json", data=data)
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
2 changes: 2 additions & 0 deletions backoffice/config/api_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from backoffice.users.api.views import UserViewSet
from backoffice.workflows.api.views import (
AuthorWorkflowViewSet,
DecisionViewSet,
WorkflowTicketViewSet,
WorkflowViewSet,
)
Expand All @@ -20,5 +21,6 @@
)
router.register("workflows", WorkflowViewSet, basename="workflows")
(router.register("workflow-ticket", WorkflowTicketViewSet, basename="workflow-ticket"),)
router.register("decisions", DecisionViewSet, basename="decisions")
app_name = "api"
urlpatterns = router.urls
Loading

0 comments on commit dc2b6fc

Please sign in to comment.