Skip to content

Commit

Permalink
Fix sync rcra profile (#613)
Browse files Browse the repository at this point in the history
* adjust docker-compose and sync rcra profile url parameters

* add RCRAInfo Environment to celery worker and beat configs

* use emanifest package new method of initiation

* remove sync method from RcraProfile model

* use RcraProfileServiceError to control what exceptions are raised from service module

* introduce SiteServiceError exception class

* add regrassion tests showing RcrainfoService sets state in __init__ by environment
  • Loading branch information
dpgraham4401 authored Oct 19, 2023
1 parent cbabb54 commit e48fdb2
Show file tree
Hide file tree
Showing 13 changed files with 111 additions and 55 deletions.
2 changes: 1 addition & 1 deletion client/src/features/profile/RcraProfile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ export function RcraProfile({ profile }: ProfileViewProps) {
variant="primary"
onClick={() =>
htApi
.get(`profile/${profile.user}/sync`)
.get(`rcra/profile/sync`)
.then((response) => {
dispatch(
addNotification({
Expand Down
8 changes: 8 additions & 0 deletions docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ services:
HT_DB_PASSWORD: ${HT_DB_PASSWORD}
HT_DB_HOST: postgres
HT_DB_PORT: ${HT_DB_PORT}
command: |
sh -c "
python manage.py makemigrations &&
python manage.py migrate &&
python manage.py loaddata dev_data.yaml &&
python manage.py runserver 0.0.0.0:8000"
depends_on:
postgres:
condition: service_healthy
Expand All @@ -48,6 +54,7 @@ services:
CELERY_BROKER_URL: ${CELERY_BROKER_URL}
CELERY_RESULT_BACKEND: ${CELERY_RESULT_BACKEND}
CELERY_LOG_LEVEL: ${CELERY_LOG_LEVEL}
HT_RCRAINFO_ENV: ${HT_RCRAINFO_ENV}
HT_DB_ENGINE: ${HT_DB_ENGINE}
HT_DB_NAME: ${HT_DB_NAME}
HT_DB_USER: ${HT_DB_USER}
Expand All @@ -69,6 +76,7 @@ services:
CELERY_BROKER_URL: ${CELERY_BROKER_URL}
CELERY_RESULT_BACKEND: ${CELERY_RESULT_BACKEND}
CELERY_LOG_LEVEL: ${CELERY_LOG_LEVEL}
HT_RCRAINFO_ENV: ${HT_RCRAINFO_ENV}
HT_DB_ENGINE: ${HT_DB_ENGINE}
HT_DB_NAME: ${HT_DB_NAME}
HT_DB_USER: ${HT_DB_USER}
Expand Down
2 changes: 1 addition & 1 deletion infra/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Contributions to the IaC should follow the general directory structure below:
│ │ ├── rest-api # Helm chart for the HTTP REST API
│ │ └── worker # Helm chart for the Celery Async worker
│ └── templates
└── terrafrom # Terraform configs for provisioning resources
└── gcp # Terraform configs for provisioning resources
├── dev # Terraform configs for the dev environment
├── modules # Resuable Terraform modules
├── org # Global Terraform configs for the organization
Expand Down
4 changes: 3 additions & 1 deletion infra/gcp/dev/.terraform.lock.hcl

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 0 additions & 7 deletions server/apps/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,6 @@ class Meta:
def __str__(self):
return f"{self.user.username}"

def sync(self):
"""Launch task to sync use profile. ToDo: remove this method"""
from apps.sites.tasks import sync_user_sites

task = sync_user_sites.delay(str(self.user.username))
return task

@property
def is_api_user(self) -> bool:
"""Returns true if the use has Rcrainfo API credentials"""
Expand Down
28 changes: 18 additions & 10 deletions server/apps/core/services/rcrainfo_service.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging
import os
from typing import Optional
from typing import Literal, Optional

import emanifest
from django.db import IntegrityError
from emanifest import RcrainfoClient, RcrainfoResponse # type: ignore

Expand All @@ -19,16 +19,25 @@ class RcrainfoService(RcrainfoClient):

datetime_format = "%Y-%m-%dT%H:%M:%S.%f%z"

def __init__(self, *, api_username: str, rcrainfo_env: Optional[str] = None, **kwargs):
def __init__(
self,
*,
api_username: str,
rcrainfo_env: Optional[Literal["preprod"] | Literal["prod"]] = None,
**kwargs,
):
self.api_user = api_username
if RcraProfile.objects.filter(user__username=self.api_user).exists():
self.profile = RcraProfile.objects.get(user__username=self.api_user)
else:
self.profile = None
if rcrainfo_env is None:
rcrainfo_env = os.getenv("HT_RCRAINFO_ENV", "preprod")
self.rcrainfo_env = rcrainfo_env
super().__init__(rcrainfo_env, **kwargs)
if rcrainfo_env == "prod":
self.rcrainfo_env: Literal["preprod"] | Literal["prod"] = rcrainfo_env
base_url = emanifest.RCRAINFO_PROD
else:
self.rcrainfo_env = "preprod"
base_url = emanifest.RCRAINFO_PREPROD
super().__init__(base_url, **kwargs)

@property
def has_api_user(self) -> bool:
Expand Down Expand Up @@ -56,15 +65,14 @@ def retrieve_key(self, api_key=None) -> str:
return super().retrieve_key(self.profile.rcra_api_key)
return super().retrieve_key()

def get_user_profile(self, username: Optional[str] = None):
def get_user_profile(self, username: Optional[str] = None) -> RcrainfoResponse:
"""
Retrieve a user's site permissions from RCRAInfo, It expects the
haztrak user to have their unique RCRAInfo user and API credentials in their
RcraProfile
"""
profile = RcraProfile.objects.get(user__username=username or self.api_user)
response = self.search_users(userId=profile.rcra_username)
return response.json()
return self.search_users(userId=profile.rcra_username)

def sync_federal_waste_codes(self):
"""
Expand Down
22 changes: 22 additions & 0 deletions server/apps/core/tests/test_rcrainfo_service.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import emanifest
import pytest

from apps.core.services import RcrainfoService


class TestRcrainfoService:
"""Tests the for the RcrainfoService class"""

@pytest.fixture
def user(self, user_factory):
return user_factory()

def test_rcrainfo_inits_to_correct_environment(self, user):
rcrainfo = RcrainfoService(api_username=user.username, rcrainfo_env="preprod")
assert rcrainfo.rcrainfo_env == "preprod"

def test_rcrainfo_inits_base_url_by_env(self, user):
rcrainfo_preprod = RcrainfoService(api_username=user.username, rcrainfo_env="preprod")
assert rcrainfo_preprod.base_url == emanifest.RCRAINFO_PREPROD
rcrainfo_prod = RcrainfoService(api_username=user.username, rcrainfo_env="prod")
assert rcrainfo_prod.base_url == emanifest.RCRAINFO_PROD
2 changes: 1 addition & 1 deletion server/apps/core/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"rcra/",
include(
[
path("profile/<str:username>/sync", RcraProfileSyncView.as_view()),
path("profile/sync", RcraProfileSyncView.as_view()),
path("profile/<str:username>", RcraProfileView.as_view()),
]
),
Expand Down
8 changes: 4 additions & 4 deletions server/apps/core/views/profile_views.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
from celery.exceptions import CeleryError
from celery.result import AsyncResult as CeleryTask
from rest_framework import status
from rest_framework.generics import GenericAPIView, RetrieveUpdateAPIView
from rest_framework.request import Request
from rest_framework.response import Response

from apps.core.models import HaztrakUser, RcraProfile
from apps.core.serializers import HaztrakUserSerializer, RcraProfileSerializer
from apps.sites.tasks import sync_user_sites


class HaztrakUserView(RetrieveUpdateAPIView):
Expand Down Expand Up @@ -38,14 +40,12 @@ class RcraProfileSyncView(GenericAPIView):
with their haztrak (Rcra)profile.
"""

queryset = RcraProfile.objects.all()
queryset = None
response = Response

def get(self, request: Request) -> Response:
"""Sync Profile GET method rcra_site"""
try:
profile = RcraProfile.objects.get(user=request.user)
task = profile.sync()
task: CeleryTask = sync_user_sites.delay(str(self.request.user))
return self.response({"task": task.id})
except RcraProfile.DoesNotExist as exc:
return self.response(data={"error": str(exc)}, status=status.HTTP_404_NOT_FOUND)
Expand Down
58 changes: 36 additions & 22 deletions server/apps/sites/services/profile_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@
from apps.sites.serializers import RcraPermissionSerializer # type: ignore

from ...core.models import RcraProfile # type: ignore
from .site_services import RcraSiteService, SiteService # type: ignore
from .site_services import RcraSiteService, SiteService, SiteServiceError # type: ignore

logger = logging.getLogger(__name__)


# ToDo, may be better to have a service level module exception.
class RcraServiceError(Exception):
class RcraProfileServiceError(Exception):
"""Exception for errors specific to the RcraProfileService"""

def __init__(self, message: str):
Expand Down Expand Up @@ -47,7 +46,7 @@ def __repr__(self):
f"<{self.__class__.__name__}(username='{self.username}', rcrainfo='{self.rcrainfo}')>"
)

def pull_rcra_profile(self, *, username: Optional[str] = None):
def pull_rcra_profile(self, *, username: Optional[str] = None) -> None:
"""
This high level function makes several requests to RCRAInfo to pull...
1. A user's rcrainfo site permissions, it creates a RcraSitePermission for each
Expand All @@ -56,30 +55,43 @@ def pull_rcra_profile(self, *, username: Optional[str] = None):
3. If a Haztrak Site is not present, create one
"""
try:
handler_service = RcraSiteService(username=self.username, rcrainfo=self.rcrainfo)
site_service = SiteService(username=self.username, rcrainfo=self.rcrainfo)
if username:
user_to_update: str = username
else:
user_to_update = self.username
user_profile_response = self.rcrainfo.get_user_profile(username=user_to_update)
permissions = self._parse_rcra_response(rcra_response=user_profile_response)
if username is None:
username = self.username
profile_response = self.rcrainfo.get_user_profile(username=username)
permissions = self._parse_rcra_response(rcra_response=profile_response.json())
self._update_profile_permissions(permissions)
except (RcraProfile.DoesNotExist, Site.DoesNotExist) as exc:
raise RcraProfileServiceError(exc)

def _update_profile_permissions(self, permissions: list[dict]):
"""
This function creates or updates a user's RcraSitePermission for each site permission
:param permissions: body of response from RCRAInfo
:return: None
"""
try:
handler = RcraSiteService(username=self.username, rcrainfo=self.rcrainfo)
haztrak_site = SiteService(username=self.username, rcrainfo=self.rcrainfo)
for rcra_site_permission in permissions:
rcra_site = handler_service.get_or_pull_rcra_site(rcra_site_permission["siteId"])
site = site_service.create_or_update_site(rcra_site=rcra_site)
rcra_site = handler.get_or_pull_rcra_site(rcra_site_permission["siteId"])
site = haztrak_site.create_or_update_site(rcra_site=rcra_site)
self._create_or_update_rcra_permission(
epa_permission=rcra_site_permission, site=site
)

except (RcraProfile.DoesNotExist, Site.DoesNotExist) as exc:
raise Exception(exc)
except SiteServiceError as exc:
raise RcraProfileServiceError(f"Error creating or updating Haztrak Site {exc}")
except KeyError as exc:
raise RcraProfileServiceError(f"Error parsing RCRAInfo response: {str(exc)}")

@staticmethod
def _parse_rcra_response(*, rcra_response: dict) -> list:
permissions = []
for permission_json in rcra_response["users"][0]["sites"]:
permissions.append(permission_json)
return permissions
try:
permissions = []
for permission_json in rcra_response["users"][0]["sites"]:
permissions.append(permission_json)
return permissions
except KeyError as exc:
raise RcraProfileServiceError(f"Error parsing RCRAInfo response: {str(exc)}")

@transaction.atomic
def _create_or_update_rcra_permission(
Expand All @@ -91,4 +103,6 @@ def _create_or_update_rcra_permission(
**permission_serializer.validated_data, site=site, profile=self.profile
)
return obj
raise Exception("Error Attempting to create RcraSitePermission")
raise RcraProfileServiceError(
f"Error creating instance of RcraSitePermission {permission_serializer.errors}"
)
10 changes: 7 additions & 3 deletions server/apps/sites/services/site_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@
logger = logging.getLogger(__name__)


class SiteServiceError(Exception):
def __init__(self, message: str):
super().__init__(message)


class SiteService:
"""
SiteService encapsulates the Haztrak site subdomain business logic and use cases.
Expand Down Expand Up @@ -47,20 +52,19 @@ def sync_rcra_manifest(self, *, site_id: Optional[str] = None) -> PullManifestsR
site_id=site_id, start_date=site.last_rcra_sync
)
# ToDo: uncomment this after we have manifest development fixtures
# limit the number of manifest to sync at a time
# limit the number of manifest to sync at a time per RCRAInfo rate limits
tracking_numbers = tracking_numbers[:30]
logger.info(f"Pulling {tracking_numbers} from RCRAInfo")
results: PullManifestsResult = manifest_service.pull_manifests(
tracking_numbers=tracking_numbers
)
# ToDo: uncomment this after we have manifest development fixtures
# Update the Rcrainfo last sync date for future sync operations
# site.last_rcra_sync = datetime.now().replace(tzinfo=timezone.utc)
site.save()
return results
except Site.DoesNotExist:
logger.warning(f"Site Does not exists {site_id}")
raise Exception
raise SiteServiceError(f"Site Does not exists {site_id}")

@transaction.atomic
def create_or_update_site(
Expand Down
7 changes: 6 additions & 1 deletion server/apps/sites/tasks/profile_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from celery.exceptions import Ignore, Reject
from requests import RequestException

from apps.sites.services.profile_services import RcraProfileServiceError

logger = logging.getLogger(__name__)


Expand All @@ -28,6 +30,9 @@ def sync_user_sites(self: RcraProfileTasks, username: str) -> None:
except (ConnectionError, RequestException, TimeoutError):
# ToDo retry if network error, see celery docs
raise Reject()
except RcraProfileServiceError as exc:
self.update_state(state=states.FAILURE, meta={"error": f"{str(exc)}"})
raise Ignore()
except Exception as exc:
self.update_state(state=states.FAILURE, meta=f"unknown error: {exc}")
self.update_state(state=states.FAILURE, meta={"unknown error": f"{str(exc)}"})
raise Ignore()
8 changes: 4 additions & 4 deletions server/apps/trak/tasks/manifest_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ def sign_manifest(
except (ConnectionError, TimeoutError) as exc:
raise Reject(exc)
except ValueError as exc:
self.update_state(state=states.FAILURE, meta=f"ValueError: {exc}")
self.update_state(state=states.FAILURE, meta={"Error": f"{repr(exc)}"})
raise Ignore()
except Exception as exc:
self.update_state(state=states.FAILURE, meta=f"unknown error: {exc}")
self.update_state(state=states.FAILURE, meta={"unknown error": f"{exc}"})
raise Ignore()


Expand All @@ -88,7 +88,7 @@ def sync_site_manifests(self, *, site_id: str, username: str):
return results
except Exception as exc:
logger.error(f"failed to sync {site_id} manifest")
self.update_state(state=states.FAILURE, meta=f"Internal Error {exc}")
self.update_state(state=states.FAILURE, meta={f"Error: {exc}"})
raise Ignore()


Expand All @@ -115,5 +115,5 @@ def create_rcra_manifest(self, *, manifest: dict, username: str):
return resp.json()
except Exception as exc:
logger.error("error: ", exc)
task_status.update_task_status(status="FAILURE", results=str(exc))
task_status.update_task_status(status="FAILURE", results={"result": str(exc)})
return {"error": f"Internal Error: {exc}"}

0 comments on commit e48fdb2

Please sign in to comment.