Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into feat/metadata-on-buff…
Browse files Browse the repository at this point in the history
…er-msg
  • Loading branch information
jimleroyer committed Feb 12, 2025
2 parents af1bb52 + 07387ba commit cfce611
Show file tree
Hide file tree
Showing 20 changed files with 211 additions and 69 deletions.
1 change: 1 addition & 0 deletions .github/workflows/build_and_push_performance_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ on:
paths:
- 'tests-perf/locust/**'
- 'tests-perf/ops/**'
- 'bin/execute_and_publish_performance_test.sh'

env:
GITHUB_SHA: ${{ github.sha }}
Expand Down
10 changes: 8 additions & 2 deletions .github/workflows/performance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,27 @@ jobs:
steps:
- name: Install libcurl
run: sudo apt-get update && sudo apt-get install libssl-dev libcurl4-openssl-dev

- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1

- name: Set up Python 3.12
uses: actions/setup-python@b64ffcaf5b410884ad320a9cfac8866006a109aa # v4.8.0
with:
python-version: '3.12'

- name: Upgrade pip
run: python -m pip install --upgrade pip
- uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3.3.3

- uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0
with:
path: ~/.cache/pip
key: ${{ runner.os }}-pip-${{ hashFiles('requirements.txt') }}
key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }}
restore-keys: |
${{ runner.os }}-pip-
- name: Run performance tests
run: /bin/bash -c "poetry install --with test && locust --headless --config tests-perf/locust/locust.conf -f tests-perf/locust/locust-notifications.py"

- name: Notify Slack channel if this performance test job fails
if: ${{ failure() && github.ref == 'refs/heads/main' }}
run: |
Expand Down
19 changes: 17 additions & 2 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,54 +20,68 @@ jobs:
steps:
- name: Install libcurl
run: sudo apt-get update && sudo apt-get install libssl-dev libcurl4-openssl-dev

- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1

- name: Set up Python 3.12
uses: actions/setup-python@b64ffcaf5b410884ad320a9cfac8866006a109aa # v4.8.0
with:
python-version: '3.12'

- name: Upgrade pip
run: python -m pip install --upgrade pip
- uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3.3.3

- uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0
with:
path: ~/.cache/pip
key: ${{ runner.os }}-pip-${{ hashFiles('requirements.txt') }}
key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }}
restore-keys: |
${{ runner.os }}-pip-
- name: Install poetry
env:
POETRY_VERSION: "1.7.1"
run: pip install poetry==${POETRY_VERSION} && poetry --version

- name: Check poetry.lock aligns with pyproject.toml
run: poetry check --lock

- name: Install requirements
run: poetry install --with test

- name: Run tests
run: poetry run make test

- name: Upload pytest logs on failure
if: ${{ failure() }}
uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0
with:
name: pytest-logs
path: |
pytest*.log
- name: Get python version
run: |
python_version=$(python -V | cut -d' ' -f2)
echo "python_version=${python_version}" >> $GITHUB_ENV
- name: Make version file
run: |
printf '__commit_sha__ = "09cfe03100443fb9071bba88d5c8775ff54a9ebc"\n__time__ = "2022-07-25:15:11:05"\n' > version.py
cp version.py "${{ github.workspace }}/app/"
- name: Copy site-packages in workspace
working-directory: ${{ github.workspace }}
shell: bash
run: |
mkdir -p "${{ github.workspace }}/env/" && cp -fR $(poetry env list | poetry env info -p)/lib/python3.12/site-packages "${{ github.workspace }}/env/"
- name: Install development .env file
working-directory: ${{ github.workspace }}
shell: bash
run: |
cp -f .env.example .env
- name: Checks for new endpoints against AWS WAF rules
uses: cds-snc/notification-utils/.github/actions/[email protected]
with:
Expand All @@ -76,6 +90,7 @@ jobs:
flask-mod: 'application'
flask-prop: 'application'
base-url: 'https://api.staging.notification.cdssandbox.xyz'

- name: Notify Slack channel if this job fails
if: ${{ failure() && github.ref == 'refs/heads/main' }}
run: |
Expand Down
13 changes: 0 additions & 13 deletions app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,6 @@ class QueueNames(object):

NORMAL = "normal-tasks"

# A queue meant for database tasks but it seems to be the default for sending
# notifications in some occasion. Need to investigate the purpose of this one
# further.
DATABASE = "database-tasks"

# database operations for high priority notifications
PRIORITY_DATABASE = "-priority-database-tasks.fifo"

Expand Down Expand Up @@ -104,17 +99,10 @@ class QueueNames(object):
RETRY = "retry-tasks"

NOTIFY = "notify-internal-tasks"
PROCESS_FTP = "process-ftp-tasks"
CREATE_LETTERS_PDF = "create-letters-pdf-tasks"
CALLBACKS = "service-callbacks"
CALLBACKS_RETRY = "service-callbacks-retry"

# Queue for letters, unused by CDS at this time as we don't use these.
LETTERS = "letter-tasks"

# Queue for antivirus/malware tasks
ANTIVIRUS = "antivirus-tasks"

# Queue for delivery receipts such as emails sent through AWS SES.
DELIVERY_RECEIPTS = "delivery-receipts"

Expand Down Expand Up @@ -142,7 +130,6 @@ def all_queues():
QueueNames.PRIORITY,
QueueNames.PERIODIC,
QueueNames.BULK,
QueueNames.DATABASE,
QueueNames.PRIORITY_DATABASE,
QueueNames.NORMAL_DATABASE,
QueueNames.BULK_DATABASE,
Expand Down
9 changes: 9 additions & 0 deletions app/dao/permissions_dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,14 @@ def get_permissions_by_user_id_and_service_id(self, user_id, service_id):
self.Meta.model.query.filter_by(user_id=user_id).join(Permission.service).filter_by(active=True, id=service_id).all()
)

def get_team_members_with_permission(self, service_id, permission):
permission_objs = (
self.Meta.model.query.filter_by(service_id=service_id, permission=permission)
.join(Permission.user)
.filter_by(state="active")
.all()
)
return [p.user for p in permission_objs]


permission_dao = PermissionDAO()
2 changes: 1 addition & 1 deletion app/dao/users_dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def verify_within_time(user, age=timedelta(seconds=30)):
return query.count()


def get_user_by_id(user_id=None):
def get_user_by_id(user_id=None) -> User:
if user_id:
return User.query.filter_by(id=user_id).one()
return User.query.filter_by().all()
Expand Down
16 changes: 16 additions & 0 deletions app/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,23 @@ def __str__(self):
return str(self.to_dict())


class CannotRemoveUserError(InvalidRequest):
message = "Cannot remove user from team"

def __init__(self, fields=[], message=None, status_code=400):
# Call parent class __init__ with message and status_code
super().__init__(message=message if message else self.message, status_code=status_code)
self.fields = fields


def register_errors(blueprint):
@blueprint.errorhandler(CannotRemoveUserError)
def cannot_remove_user_error(error):
response = jsonify(error.to_dict())
response.status_code = error.status_code
current_app.logger.info(error.message)
return response

@blueprint.errorhandler(InvalidEmailError)
def invalid_format(error):
# Please not that InvalidEmailError is re-raised for InvalidEmail or InvalidPhone,
Expand Down
7 changes: 7 additions & 0 deletions app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,13 @@ def serialize_for_org_dashboard(self) -> dict:
"research_mode": self.research_mode,
}

def get_users_with_permission(self, permission):
from app.dao.permissions_dao import permission_dao

if permission:
return permission_dao.get_team_members_with_permission(self.id, permission)
return []


class AnnualBilling(BaseModel):
__tablename__ = "annual_billing"
Expand Down
15 changes: 13 additions & 2 deletions app/service/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,12 @@
)
from app.dao.templates_dao import dao_get_template_by_id
from app.dao.users_dao import get_user_by_id
from app.errors import InvalidRequest, register_errors
from app.errors import CannotRemoveUserError, InvalidRequest, register_errors
from app.models import (
EMAIL_TYPE,
KEY_TYPE_NORMAL,
LETTER_TYPE,
MANAGE_SETTINGS,
NOTIFICATION_CANCELLED,
SMS_TYPE,
EmailBranding,
Expand Down Expand Up @@ -490,13 +491,23 @@ def add_user_to_service(service_id, user_id):
def remove_user_from_service(service_id, user_id):
service = dao_fetch_service_by_id(service_id)
user = get_user_by_id(user_id=user_id)
users_with_manage_settings_perm = service.get_users_with_permission(MANAGE_SETTINGS)

if user not in service.users:
error = "User not found"
raise InvalidRequest(error, status_code=404)

elif len(service.users) == 1:
error = "You cannot remove the only user for a service"
raise InvalidRequest(error, status_code=400)
raise CannotRemoveUserError(message=error)

elif len(service.users) == 2:
error = "SERVICE_CANNOT_HAVE_LT_2_MEMBERS"
raise CannotRemoveUserError(message=error)

elif user in users_with_manage_settings_perm and len(users_with_manage_settings_perm) <= 1:
error = "SERVICE_NEEDS_USER_W_MANAGE_SETTINGS_PERM"
raise CannotRemoveUserError(message=error)

dao_remove_user_from_service(service, user)

Expand Down
37 changes: 1 addition & 36 deletions app/v2/notifications/post_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -597,42 +597,7 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text


def process_precompiled_letter_notifications(*, letter_data, api_key, template, reply_to_text):
try:
status = NOTIFICATION_PENDING_VIRUS_CHECK
letter_content = base64.b64decode(letter_data["content"])
except ValueError:
raise BadRequestError(
message="Cannot decode letter content (invalid base64 encoding)",
status_code=400,
)

notification = create_letter_notification(
letter_data=letter_data,
template=template,
api_key=api_key,
status=status,
reply_to_text=reply_to_text,
)

filename = upload_letter_pdf(notification, letter_content, precompiled=True)

current_app.logger.info("Calling task scan-file for {}".format(filename))

# call task to add the filename to anti virus queue
if current_app.config["ANTIVIRUS_ENABLED"]:
notify_celery.send_task(
name=TaskNames.SCAN_FILE,
kwargs={"filename": filename},
queue=QueueNames.ANTIVIRUS,
)
else:
# stub out antivirus in dev
process_virus_scan_passed.apply_async(
kwargs={"filename": filename},
queue=QueueNames.LETTERS,
)

return notification
pass


def validate_sender_id(template, reply_to_id):
Expand Down
12 changes: 11 additions & 1 deletion bin/execute_and_publish_performance_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ mkdir -p "$perf_test_results_folder"

# Run old performance test and copy results to S3
locust --headless --config tests-perf/locust/locust.conf --html "$perf_test_results_folder/index.html" --csv "$perf_test_results_folder/perf_test"
aws s3 cp $"perf_test_csv_directory_path/" "s3://$perf_test_aws_s3_bucket" --recursive || exit 1
aws s3 cp "$perf_test_csv_directory_path/" "s3://$perf_test_aws_s3_bucket" --recursive || exit 1

# Sleep 15 minutes to allow the system to stabilize
sleep 900
Expand All @@ -23,5 +23,15 @@ if [ "$(date +%u)" -ge 2 ] && [ "$(date +%u)" -le 5 ]; then
locust --headless --host https://api.staging.notification.cdssandbox.xyz --locustfile tests-perf/locust/send_rate_email.py --users 5 --run-time 10m --spawn-rate 1
fi

# Sleep 30 minutes to allow the tests to finish and the system to stabilize
sleep 1800

# Run sms send rate performance test
# This configuration should send 4K sms / minute for 5 minutes for 20K sms total.
# We run this test on Tuesday through Friday (just after midnight UTC) only.
if [ "$(date +%u)" -ge 2 ] && [ "$(date +%u)" -le 5 ]; then
locust --headless --host https://api.staging.notification.cdssandbox.xyz --locustfile tests-perf/locust/send_rate_sms.py --users 2 --run-time 5m --spawn-rate 1
fi

# Cleanup
rm -rf "${perf_test_csv_directory_path:?}/${current_time:?}"
2 changes: 1 addition & 1 deletion scripts/run_celery.ps1
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
$ENV:FORKED_BY_MULTIPROCESSING=1

celery --app run_celery worker --pidfile="$env:TEMP\celery.pid" --pool=solo --loglevel=DEBUG --concurrency=1 -Q "database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-high,send-sms-medium,send-sms-low,service-callbacks,service-callbacks-retry,delivery-receipts"
celery --app run_celery worker --pidfile="$env:TEMP\celery.pid" --pool=solo --loglevel=DEBUG --concurrency=1 -Q "-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-high,send-sms-medium,send-sms-low,service-callbacks,service-callbacks-retry,delivery-receipts"
2 changes: 1 addition & 1 deletion scripts/run_celery.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ set -e

echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"

celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-high,send-sms-medium,send-sms-low,service-callbacks,service-callbacks-retry,delivery-receipts
celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q -priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-high,send-sms-medium,send-sms-low,service-callbacks,service-callbacks-retry,delivery-receipts
2 changes: 1 addition & 1 deletion scripts/run_celery_core_tasks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ set -e

echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"

celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,service-callbacks,service-callbacks-retry,delivery-receipts
celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q -priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,service-callbacks,service-callbacks-retry,delivery-receipts
2 changes: 1 addition & 1 deletion scripts/run_celery_local.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ set -e

echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"

celery -A run_celery.notify_celery worker --beat --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-high,send-sms-medium,send-sms-low,send-throttled-sms-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,service-callbacks-retry,delivery-receipts
celery -A run_celery.notify_celery worker --beat --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q -priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-high,send-sms-medium,send-sms-low,send-throttled-sms-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,service-callbacks-retry,delivery-receipts
2 changes: 1 addition & 1 deletion scripts/run_celery_no_sms_sending.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ fi

echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"

celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,service-callbacks-retry,delivery-receipts
celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q -priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,service-callbacks-retry,delivery-receipts
Loading

0 comments on commit cfce611

Please sign in to comment.