From 943c377f138c233298e92de1ce7563b4f37d20e7 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Wed, 17 Jan 2024 13:47:26 -0900 Subject: [PATCH 001/115] Create deploy-credits-sandbox.yml --- .github/workflows/deploy-credits-sandbox.yml | 75 ++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 .github/workflows/deploy-credits-sandbox.yml diff --git a/.github/workflows/deploy-credits-sandbox.yml b/.github/workflows/deploy-credits-sandbox.yml new file mode 100644 index 000000000..72bc80285 --- /dev/null +++ b/.github/workflows/deploy-credits-sandbox.yml @@ -0,0 +1,75 @@ +name: Deploy Credits Sandbox Stack to AWS + +on: + push: + branches: + - credits-sandbox + +concurrency: ${{ github.workflow }}-${{ github.ref }} + +jobs: + deploy: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - environment: hyp3-credits-sandbox + domain: hyp3-credits-sandbox.asf.alaska.edu + template_bucket: cf-templates-1hz9ldhhl4ahu-us-west-2 + image_tag: test + product_lifetime_in_days: 14 + quota: 0 + deploy_ref: refs/heads/credits-sandbox + job_files: job_spec/AUTORIFT.yml job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml job_spec/INSAR_ISCE_BURST.yml + instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge + default_max_vcpus: 640 + expanded_max_vcpus: 640 + required_surplus: 0 + security_environment: ASF + ami_id: /aws/service/ecs/optimized-ami/amazon-linux-2023/recommended/image_id + distribution_url: '' + + environment: + name: ${{ matrix.environment }} + url: https://${{ matrix.domain }} + + steps: + - uses: actions/checkout@v4.1.1 + + - uses: aws-actions/configure-aws-credentials@v4 + with: + aws-access-key-id: ${{ secrets.V2_AWS_ACCESS_KEY_ID }} + aws-secret-access-key: ${{ secrets.V2_AWS_SECRET_ACCESS_KEY }} + aws-session-token: ${{ secrets.V2_AWS_SESSION_TOKEN }} + aws-region: ${{ secrets.AWS_REGION }} + + - uses: actions/setup-python@v5 + with: + python-version: 3.9 + + - uses: ./.github/actions/deploy-hyp3 + with: + TEMPLATE_BUCKET: ${{ matrix.template_bucket }} + STACK_NAME: ${{ matrix.environment }} + DOMAIN_NAME: ${{ matrix.domain }} + API_NAME: ${{ matrix.environment }} + CERTIFICATE_ARN: ${{ secrets.CERTIFICATE_ARN }} + IMAGE_TAG: ${{ matrix.image_tag }} + PRODUCT_LIFETIME: ${{ matrix.product_lifetime_in_days }} + VPC_ID: ${{ secrets.VPC_ID }} + SUBNET_IDS: ${{ secrets.SUBNET_IDS }} + SECRET_ARN: ${{ secrets.SECRET_ARN }} + CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} + MONTHLY_JOB_QUOTA_PER_USER: ${{ matrix.quota }} + JOB_FILES: ${{ matrix.job_files }} + DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} + EXPANDED_MAX_VCPUS: ${{ matrix.expanded_max_vcpus }} + MONTHLY_BUDGET: ${{ secrets.MONTHLY_BUDGET }} + REQUIRED_SURPLUS: ${{ matrix.required_surplus }} + ORIGIN_ACCESS_IDENTITY_ID: ${{ secrets.ORIGIN_ACCESS_IDENTITY_ID }} + SECURITY_ENVIRONMENT: ${{ matrix.security_environment }} + AMI_ID: ${{ matrix.ami_id }} + INSTANCE_TYPES: ${{ matrix.instance_types }} + DISTRIBUTION_URL: ${{ matrix.distribution_url }} + AUTH_PUBLIC_KEY: ${{ secrets.AUTH_PUBLIC_KEY }} From 9037514cb8362c70fa694b7507b659ba2c48a2d5 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Thu, 18 Jan 2024 15:26:14 -0900 Subject: [PATCH 002/115] pseudocode implementing credit system for POST /jobs API endpoint --- apps/api/src/hyp3_api/handlers.py | 11 ++-- lib/dynamo/dynamo/jobs.py | 102 +++++++++--------------------- lib/dynamo/dynamo/user.py | 11 ---- 3 files changed, 36 insertions(+), 88 deletions(-) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 929346ab9..c316d3569 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -41,12 +41,11 @@ def post_jobs(body, user): except GranuleValidationError as e: abort(problem_format(400, str(e))) - if not body.get('validate_only'): - try: - body['jobs'] = dynamo.jobs.put_jobs(user, body['jobs']) - except dynamo.jobs.QuotaError as e: - abort(problem_format(400, str(e))) - return body + try: + body['jobs'] = dynamo.jobs.put_jobs(user, body['jobs'], validate_only=body.get('validate_only')) + except dynamo.jobs.InsufficientCreditsError as e: + abort(problem_format(400, str(e))) + return body def get_jobs(user, start=None, end=None, status_code=None, name=None, job_type=None, start_token=None): diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 7281990eb..458ccb414 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -1,6 +1,6 @@ from datetime import datetime, timezone from os import environ -from typing import Callable, List, Optional, Union +from typing import List from uuid import uuid4 from boto3.dynamodb.conditions import Attr, Key @@ -9,95 +9,55 @@ from dynamo.util import DYNAMODB_RESOURCE, convert_floats_to_decimals, format_time, get_request_time_expression -class QuotaError(Exception): +class InsufficientCreditsError(Exception): """Raised when trying to submit more jobs that user has remaining""" -def _get_job_count_for_month(user) -> int: - now = datetime.now(timezone.utc) - start_of_month = now.replace(day=1, hour=0, minute=0, second=0, microsecond=0) - job_count_for_month = count_jobs(user, format_time(start_of_month)) - return job_count_for_month +def get_cost(job: dict) -> float: + return 1.0 -def get_quota_status(user) -> Union[tuple[int, int, int], tuple[None, None, None]]: - max_jobs = dynamo.user.get_max_jobs_per_month(user) - - if max_jobs is not None: - previous_jobs = _get_job_count_for_month(user) - remaining_jobs = max(max_jobs - previous_jobs, 0) - else: - previous_jobs = None - remaining_jobs = None - - return max_jobs, previous_jobs, remaining_jobs - - -def put_jobs(user_id: str, jobs: List[dict], fail_when_over_quota=True) -> List[dict]: +def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: table = DYNAMODB_RESOURCE.Table(environ['JOBS_TABLE_NAME']) request_time = format_time(datetime.now(timezone.utc)) - max_jobs, previous_jobs, remaining_jobs = get_quota_status(user_id) - has_quota = max_jobs is not None - if has_quota: - assert previous_jobs is not None - assert remaining_jobs is not None - - if has_quota and len(jobs) > remaining_jobs: - if fail_when_over_quota: - raise QuotaError(f'Your monthly quota is {max_jobs} jobs. You have {remaining_jobs} jobs remaining.') - jobs = jobs[:remaining_jobs] + try: # TODO deal with the new user use case + user = dynamo.user.get_user(user_id) + except NotFoundError: + user = dynamo.create_user(user_id) - priority_override = dynamo.user.get_priority(user_id) - priority = _get_job_priority(priority_override, has_quota) + remaining_credits = user['credits'] + priority_override = user.get('priority_override') - prepared_jobs = [ - { + prepared_jobs = [] + for job in jobs: + prepared_job = { 'job_id': str(uuid4()), 'user_id': user_id, 'status_code': 'PENDING', 'execution_started': False, 'request_time': request_time, - 'priority': priority(previous_jobs, index), + 'cost': get_cost(job), **job, - } for index, job in enumerate(jobs) - ] - - for prepared_job in prepared_jobs: - table.put_item(Item=convert_floats_to_decimals(prepared_job)) + } + if priority_override: + prepared_job['priority'] = priority_override + else: + priority = max(prepared_jobs[-1]['priority'] - prepared_job['cost'], 0) + prepared_job['priority'] = priority + prepared_jobs.append(prepared_job) + + total_cost = sum([job['cost'] for job in prepared_jobs]) + if remaining_credits is not None and total_cost > remaining_credits: + raise InsufficientCreditsError(f'These jobs would cost {total_cost} remaining_credits, but you have only {remaining_credits} remaining.') # TODO change exception name + + if not dry_run: + for prepared_job in prepared_jobs: + table.put_item(Item=convert_floats_to_decimals(prepared_job)) + dynamo.user.decrement_credits(total_cost) return prepared_jobs -def _get_job_priority(priority_override: Optional[int], has_quota: bool) -> Callable[[Optional[int], int], int]: - if priority_override is not None: - priority = lambda _, __: priority_override - elif has_quota: - priority = lambda previous_jobs, job_index: max(9999 - previous_jobs - job_index, 0) - else: - priority = lambda _, __: 0 - return priority - - -def count_jobs(user, start=None, end=None): - table = DYNAMODB_RESOURCE.Table(environ['JOBS_TABLE_NAME']) - key_expression = Key('user_id').eq(user) - if start is not None or end is not None: - key_expression &= get_request_time_expression(start, end) - - params = { - 'IndexName': 'user_id', - 'KeyConditionExpression': key_expression, - 'Select': 'COUNT', - } - response = table.query(**params) - job_count = response['Count'] - while 'LastEvaluatedKey' in response: - params['ExclusiveStartKey'] = response['LastEvaluatedKey'] - response = table.query(**params) - job_count += response['Count'] - return job_count - - def query_jobs(user, start=None, end=None, status_code=None, name=None, job_type=None, start_key=None): table = DYNAMODB_RESOURCE.Table(environ['JOBS_TABLE_NAME']) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index c2ceebc1d..9c6691521 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -17,14 +17,3 @@ def get_priority(user_id: str) -> Optional[int]: else: priority = None return priority - - -def get_max_jobs_per_month(user_id: str) -> Optional[int]: - user = get_user(user_id) - if user is not None and 'max_jobs_per_month' in user: - max_jobs_per_month = user['max_jobs_per_month'] - if max_jobs_per_month is not None: - max_jobs_per_month = int(max_jobs_per_month) - else: - max_jobs_per_month = int(environ['MONTHLY_JOB_QUOTA_PER_USER']) - return max_jobs_per_month From c12aca2cfbbe5edb43e2f6f162af85a1f5abcdfd Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 18 Jan 2024 16:45:23 -0900 Subject: [PATCH 003/115] implement create_user and decrement_credits --- lib/dynamo/dynamo/jobs.py | 8 ++++---- lib/dynamo/dynamo/user.py | 38 +++++++++++++++++++++++++++++++------- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 458ccb414..5d40dcf12 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -21,9 +21,8 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: table = DYNAMODB_RESOURCE.Table(environ['JOBS_TABLE_NAME']) request_time = format_time(datetime.now(timezone.utc)) - try: # TODO deal with the new user use case - user = dynamo.user.get_user(user_id) - except NotFoundError: + user = dynamo.user.get_user(user_id) + if not user: user = dynamo.create_user(user_id) remaining_credits = user['credits'] @@ -54,7 +53,8 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: if not dry_run: for prepared_job in prepared_jobs: table.put_item(Item=convert_floats_to_decimals(prepared_job)) - dynamo.user.decrement_credits(total_cost) + # TODO: handle "negative balance" ValueError, which indicates a race condition + dynamo.user.decrement_credits(user, total_cost) return prepared_jobs diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 9c6691521..61930a820 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -1,6 +1,9 @@ +import os +from decimal import Decimal from os import environ from typing import Optional +import botocore.exceptions from dynamo.util import DYNAMODB_RESOURCE @@ -10,10 +13,31 @@ def get_user(user_id: str) -> Optional[dict]: return response.get('Item') -def get_priority(user_id: str) -> Optional[int]: - user = get_user(user_id) - if user: - priority = user.get('priority') - else: - priority = None - return priority +# TODO unit tests +def create_user(user_id: str) -> dict: + table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) + if get_user(user_id): + raise ValueError(f'user {user_id} already exists') + user = {'user_id': user_id, 'credits': Decimal(os.environ['MONTHLY_JOB_QUOTA_PER_USER'])} + table.put_item(Item=user) + return user + + +# TODO unit tests +def decrement_credits(user_id: str, cost: float) -> None: + if cost < 0: + raise ValueError(f'Cost {cost} < 0') + table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) + try: + table.update_item( + Key={'user_id': user_id}, + UpdateExpression='ADD credits :delta', + ConditionExpression='credits >= :cost', + ExpressionAttributeValues={':cost': cost, ':delta': -cost}, + ) + except botocore.exceptions.ClientError as e: + if e.response['Error']['Code'] == 'ConditionalCheckFailedException': + raise ValueError( + f'Subtracting cost {cost} from user\'s remaining credits would result in a negative balance' + ) + raise From 58718d495a1ee807d1cedda8a69cbe74e2a2a1e2 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 19 Jan 2024 09:51:05 -0900 Subject: [PATCH 004/115] update api spec with new credits schema --- .../src/hyp3_api/api-spec/openapi-spec.yml.j2 | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/apps/api/src/hyp3_api/api-spec/openapi-spec.yml.j2 b/apps/api/src/hyp3_api/api-spec/openapi-spec.yml.j2 index 26135d69c..b9ee46a0a 100644 --- a/apps/api/src/hyp3_api/api-spec/openapi-spec.yml.j2 +++ b/apps/api/src/hyp3_api/api-spec/openapi-spec.yml.j2 @@ -133,36 +133,24 @@ components: $ref: "#/components/schemas/next_url" user: - description: Information about a user (quota, user id) + description: Information about a user type: object required: - user_id - - quota + - remaining_credits additionalProperties: false properties: user_id: $ref: "#/components/schemas/user_id" - quota: - $ref: "#/components/schemas/quota" + remaining_credits: + $ref: "#/components/schemas/credits" job_names: $ref: "#/components/schemas/job_names_list" - quota: - description: Containes the limit of jobs per month and the amount remaining for a user. - type: object - required: - - max_jobs_per_month - - remaining - additionalProperties: false - properties: - max_jobs_per_month: - type: integer - minimum: 0 - nullable: true - remaining: - type: integer - minimum: 0 - nullable: true + credits: + description: Processing credits for running jobs. + type: number + minimum: 0 job_names_list: type: array @@ -287,6 +275,8 @@ components: $ref: "#/components/schemas/processing_times" priority: $ref: "#/components/schemas/priority" + credit_cost: + $ref: "#/components/schemas/credits" validate_only: type: boolean From f134d5783d276a2ca98e9059a520e8059d41fb07 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 10:03:12 -0900 Subject: [PATCH 005/115] delete obsolete dynamo.jobs tests --- lib/dynamo/dynamo/jobs.py | 3 +- tests/test_dynamo/test_jobs.py | 149 +-------------------------------- 2 files changed, 6 insertions(+), 146 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 5d40dcf12..eea896b98 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -17,13 +17,14 @@ def get_cost(job: dict) -> float: return 1.0 +# TODO add/update tests def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: table = DYNAMODB_RESOURCE.Table(environ['JOBS_TABLE_NAME']) request_time = format_time(datetime.now(timezone.utc)) user = dynamo.user.get_user(user_id) if not user: - user = dynamo.create_user(user_id) + user = dynamo.user.create_user(user_id) remaining_credits = user['credits'] priority_override = user.get('priority_override') diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 65a8939d9..4547a7a52 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -8,77 +8,6 @@ import dynamo -def test_count_jobs(tables): - table_items = [ - { - 'job_id': 'job1', - 'user_id': 'user1', - 'status_code': 'status1', - 'request_time': '2000-01-01T00:00:00+00:00', - }, - { - 'job_id': 'job2', - 'user_id': 'user1', - 'status_code': 'status1', - 'request_time': '2000-01-01T00:00:00+00:00', - }, - { - 'job_id': 'job3', - 'user_id': 'user2', - 'status_code': 'status1', - 'request_time': '2000-01-01T00:00:00+00:00', - }, - ] - for item in table_items: - tables.jobs_table.put_item(Item=item) - - assert dynamo.jobs.count_jobs('user1') == 2 - assert dynamo.jobs.count_jobs('user2') == 1 - - -def test_count_jobs_by_start(tables): - table_items = [ - { - 'job_id': 'job1', - 'user_id': 'user1', - 'status_code': 'status1', - 'request_time': '2000-01-01T00:00:00+00:00', - }, - { - 'job_id': 'job2', - 'user_id': 'user1', - 'status_code': 'status1', - 'request_time': '2000-01-02T00:00:00+00:00', - }, - { - 'job_id': 'job3', - 'user_id': 'user1', - 'status_code': 'status1', - 'request_time': '2000-01-03T00:00:00+00:00', - }, - ] - for item in table_items: - tables.jobs_table.put_item(Item=item) - - start = '2000-01-01T00:00:00+00:00' - end = '2000-01-03T00:00:00+00:00' - response = dynamo.jobs.count_jobs('user1', start, end) - assert response == 3 - - start = '2000-01-01T00:00:01+00:00' - end = '2000-01-02T00:59:59+00:00' - response = dynamo.jobs.count_jobs('user1', start, end) - assert response == 1 - - start = '2000-01-01T00:00:01+00:00' - response = dynamo.jobs.count_jobs('user1', start, None) - assert response == 2 - - end = '2000-01-02T00:59:59+00:00' - response = dynamo.jobs.count_jobs('user1', None, end) - assert response == 2 - - def test_query_jobs_by_user(tables): table_items = [ { @@ -280,6 +209,7 @@ def test_put_jobs(tables): assert response['Items'] == jobs +# TODO update def test_put_jobs_no_quota(tables, monkeypatch): monkeypatch.setenv('MONTHLY_JOB_QUOTA_PER_USER', '1') payload = [{'name': 'name1'}, {'name': 'name2'}] @@ -296,6 +226,7 @@ def test_put_jobs_no_quota(tables, monkeypatch): assert job['priority'] == 0 +# TODO update def test_put_jobs_priority_override(tables): payload = [{'name': 'name1'}, {'name': 'name2'}] tables.users_table.put_item(Item={'user_id': 'user1', 'priority': 100}) @@ -315,6 +246,7 @@ def test_put_jobs_priority_override(tables): assert job['priority'] == 550 +# TODO update def test_put_jobs_priority(tables): jobs = [] jobs.extend(dynamo.jobs.put_jobs('user1', [{}])) @@ -326,6 +258,7 @@ def test_put_jobs_priority(tables): assert jobs[3]['priority'] == 9999 +# TODO update def test_put_jobs_priority_overflow(tables, monkeypatch): monkeypatch.setenv('MONTHLY_JOB_QUOTA_PER_USER', '10001') many_jobs = [{} for ii in range(10001)] @@ -493,27 +426,6 @@ def test_get_jobs_waiting_for_execution(tables): assert dynamo.jobs.get_jobs_waiting_for_execution(limit=6) == [items[0], items[1], items[4], items[6], items[9]] -def test_put_jobs_exceeds_quota(tables): - tables.users_table.put_item(Item={'user_id': 'user1', 'max_jobs_per_month': 3}) - - dynamo.jobs.put_jobs('user1', [{}, {}, {}]) - assert dynamo.jobs.count_jobs('user1') == 3 - - with pytest.raises(dynamo.jobs.QuotaError): - dynamo.jobs.put_jobs('user1', [{}]) - assert dynamo.jobs.count_jobs('user1') == 3 - - dynamo.jobs.put_jobs('user2', [{} for i in range(25)]) - assert dynamo.jobs.count_jobs('user2') == 25 - - with pytest.raises(dynamo.jobs.QuotaError): - dynamo.jobs.put_jobs('user3', [{} for i in range(26)]) - - results = dynamo.jobs.put_jobs('user4', [{} for i in range(26)], fail_when_over_quota=False) - assert dynamo.jobs.count_jobs('user4') == 25 - assert len(results) == 25 - - def test_decimal_conversion(tables): table_items = [ { @@ -551,56 +463,3 @@ def test_decimal_conversion(tables): assert response[0]['float_value'] == Decimal('30.04') assert response[1]['float_value'] == Decimal('0.0') assert response[2]['float_value'] == Decimal('0.1') - - -def test_get_job_priority(): - priority = dynamo.jobs._get_job_priority(priority_override=None, has_quota=True) - assert priority(0, 0) == 9999 - assert priority(1, 8) == 9990 - assert priority(0, 9998) == 1 - assert priority(0, 9999) == 0 - assert priority(0, 10000) == 0 - - with pytest.raises(TypeError, match=r".*NoneType.*"): - priority(None, 9) - - priority = dynamo.jobs._get_job_priority(priority_override=1, has_quota=True) - assert priority(0, 0) == 1 - assert priority(1, 8) == 1 - assert priority(0, 9998) == 1 - assert priority(0, 9999) == 1 - assert priority(0, 10000) == 1 - - priority = dynamo.jobs._get_job_priority(priority_override=None, has_quota=False) - assert priority(None, 0) == 0 - assert priority(1, 8) == 0 - assert priority(None, 9998) == 0 - assert priority(None, 9999) == 0 - assert priority(None, 10000) == 0 - - priority = dynamo.jobs._get_job_priority(priority_override=2, has_quota=False) - assert priority(None, 0) == 2 - assert priority(1, 8) == 2 - assert priority(None, 9998) == 2 - assert priority(None, 9999) == 2 - assert priority(None, 10000) == 2 - - -@patch('dynamo.jobs._get_job_count_for_month') -@patch('dynamo.user.get_max_jobs_per_month') -def test_get_quota_status(mock_get_max_jobs_per_month: MagicMock, mock_get_job_count_for_month: MagicMock): - mock_get_max_jobs_per_month.return_value = 5 - mock_get_job_count_for_month.return_value = 0 - assert dynamo.jobs.get_quota_status('user1') == (5, 0, 5) - - mock_get_job_count_for_month.return_value = 1 - assert dynamo.jobs.get_quota_status('user1') == (5, 1, 4) - - mock_get_job_count_for_month.return_value = 10 - assert dynamo.jobs.get_quota_status('user1') == (5, 10, 0) - - mock_get_max_jobs_per_month.return_value = None - assert dynamo.jobs.get_quota_status('user1') == (None, None, None) - - assert mock_get_max_jobs_per_month.mock_calls == [call('user1') for _ in range(4)] - assert mock_get_job_count_for_month.mock_calls == [call('user1') for _ in range(3)] From 85dd723ae20540a7f40f9bf047a3411f351bf9db Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 10:04:18 -0900 Subject: [PATCH 006/115] add todo --- tests/test_dynamo/test_jobs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 4547a7a52..af227c09f 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -182,6 +182,7 @@ def test_query_jobs_by_type(tables): assert list_have_same_elements(response, table_items[:2]) +# TODO update def test_put_jobs(tables): payload = [ { From 85edf3b52dc310719a6c9a6fee22850537acfb69 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 10:10:30 -0900 Subject: [PATCH 007/115] delete obsolete dynamo.user tests --- tests/test_dynamo/test_user.py | 30 +----------------------------- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index a6f28e12f..1366e6c42 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -1,6 +1,7 @@ import dynamo +# TODO update these field names? def test_get_user(tables): table_items = [ { @@ -18,32 +19,3 @@ def test_get_user(tables): assert dynamo.user.get_user('user1') == table_items[0] assert dynamo.user.get_user('user2') == table_items[1] assert dynamo.user.get_user('foo') is None - - -def test_get_max_jobs_per_month(tables, monkeypatch): - monkeypatch.setenv('MONTHLY_JOB_QUOTA_PER_USER', '5') - assert dynamo.user.get_max_jobs_per_month('user1') == 5 - - user = {'user_id': 'user1'} - tables.users_table.put_item(Item=user) - assert dynamo.user.get_max_jobs_per_month('user1') == 5 - - user['max_jobs_per_month'] = 10 - tables.users_table.put_item(Item=user) - assert dynamo.user.get_max_jobs_per_month('user1') == 10 - - user['max_jobs_per_month'] = None - tables.users_table.put_item(Item=user) - assert dynamo.user.get_max_jobs_per_month('user1') is None - - -def test_get_priority(tables): - assert dynamo.user.get_priority('user1') is None - - user = {'user_id': 'user1'} - tables.users_table.put_item(Item=user) - assert dynamo.user.get_priority('user1') is None - - user['priority'] = 10 - tables.users_table.put_item(Item=user) - assert dynamo.user.get_priority('user1') == 10 From 14f93d11d28a79d1eca94f039a5c500605fb76e8 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 19 Jan 2024 10:39:12 -0900 Subject: [PATCH 008/115] upate get_user handler for new user schema --- apps/api/src/hyp3_api/handlers.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index c316d3569..5aeef94e9 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -79,13 +79,12 @@ def get_names_for_user(user): def get_user(user): - max_jobs, _, remaining_jobs = dynamo.jobs.get_quota_status(user) + user_record = dynamo.user.get_user(user) + if not user_record: + user_record = dynamo.user.create_user(user) return { 'user_id': user, - 'quota': { - 'max_jobs_per_month': max_jobs, - 'remaining': remaining_jobs, - }, + 'remaining_credits': user_record['remaining_credits'], 'job_names': get_names_for_user(user) } From 2c5b8026fd993ea6e1fd8fab362f5573e6a5fe57 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 19 Jan 2024 10:48:04 -0900 Subject: [PATCH 009/115] fix typo --- apps/api/src/hyp3_api/handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 5aeef94e9..7b7ee384c 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -42,7 +42,7 @@ def post_jobs(body, user): abort(problem_format(400, str(e))) try: - body['jobs'] = dynamo.jobs.put_jobs(user, body['jobs'], validate_only=body.get('validate_only')) + body['jobs'] = dynamo.jobs.put_jobs(user, body['jobs'], dry_run=body.get('validate_only')) except dynamo.jobs.InsufficientCreditsError as e: abort(problem_format(400, str(e))) return body From 35d334d5eb8d252d0e3192c12552ca374ee63e36 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 19 Jan 2024 11:10:54 -0900 Subject: [PATCH 010/115] address setting priority for first job in batch --- lib/dynamo/dynamo/jobs.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index eea896b98..1d65e2462 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -13,7 +13,7 @@ class InsufficientCreditsError(Exception): """Raised when trying to submit more jobs that user has remaining""" -def get_cost(job: dict) -> float: +def get_credit_cost(job: dict) -> float: return 1.0 @@ -37,14 +37,16 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: 'status_code': 'PENDING', 'execution_started': False, 'request_time': request_time, - 'cost': get_cost(job), + 'credit_cost': get_credit_cost(job), **job, } if priority_override: - prepared_job['priority'] = priority_override + priority = priority_override + elif prepared_jobs: + priority = max(prepared_jobs[-1]['priority'] - int(prepared_job['cost']), 0) else: - priority = max(prepared_jobs[-1]['priority'] - prepared_job['cost'], 0) - prepared_job['priority'] = priority + priority = min(int(remaining_credits), 9999) + prepared_job['priority'] = priority prepared_jobs.append(prepared_job) total_cost = sum([job['cost'] for job in prepared_jobs]) From 17a2ded6fadce3a1fc42a7825acd190a1ffac4ca Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 19 Jan 2024 11:14:36 -0900 Subject: [PATCH 011/115] more priority refactoring --- lib/dynamo/dynamo/jobs.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 1d65e2462..36b4fcb80 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -42,6 +42,8 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: } if priority_override: priority = priority_override + elif remaining_credits is None: + priority = 0 elif prepared_jobs: priority = max(prepared_jobs[-1]['priority'] - int(prepared_job['cost']), 0) else: @@ -51,7 +53,9 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: total_cost = sum([job['cost'] for job in prepared_jobs]) if remaining_credits is not None and total_cost > remaining_credits: - raise InsufficientCreditsError(f'These jobs would cost {total_cost} remaining_credits, but you have only {remaining_credits} remaining.') # TODO change exception name + raise InsufficientCreditsError( + f'These jobs would cost {total_cost} credits, but you have only {remaining_credits} remaining.' + ) if not dry_run: for prepared_job in prepared_jobs: From a65c8e6d9208e78a95aab4a3ac3935502463bf84 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 11:22:20 -0900 Subject: [PATCH 012/115] fix some key errors --- lib/dynamo/dynamo/jobs.py | 6 +++--- lib/dynamo/dynamo/user.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 1d65e2462..487fb3279 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -26,7 +26,7 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: if not user: user = dynamo.user.create_user(user_id) - remaining_credits = user['credits'] + remaining_credits = user['remaining_credits'] priority_override = user.get('priority_override') prepared_jobs = [] @@ -43,13 +43,13 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: if priority_override: priority = priority_override elif prepared_jobs: - priority = max(prepared_jobs[-1]['priority'] - int(prepared_job['cost']), 0) + priority = max(prepared_jobs[-1]['priority'] - int(prepared_job['credit_cost']), 0) else: priority = min(int(remaining_credits), 9999) prepared_job['priority'] = priority prepared_jobs.append(prepared_job) - total_cost = sum([job['cost'] for job in prepared_jobs]) + total_cost = sum([job['credit_cost'] for job in prepared_jobs]) if remaining_credits is not None and total_cost > remaining_credits: raise InsufficientCreditsError(f'These jobs would cost {total_cost} remaining_credits, but you have only {remaining_credits} remaining.') # TODO change exception name diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 61930a820..bc54480a2 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -18,7 +18,7 @@ def create_user(user_id: str) -> dict: table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) if get_user(user_id): raise ValueError(f'user {user_id} already exists') - user = {'user_id': user_id, 'credits': Decimal(os.environ['MONTHLY_JOB_QUOTA_PER_USER'])} + user = {'user_id': user_id, 'remaining_credits': Decimal(os.environ['MONTHLY_JOB_QUOTA_PER_USER'])} table.put_item(Item=user) return user @@ -31,8 +31,8 @@ def decrement_credits(user_id: str, cost: float) -> None: try: table.update_item( Key={'user_id': user_id}, - UpdateExpression='ADD credits :delta', - ConditionExpression='credits >= :cost', + UpdateExpression='ADD remaining_credits :delta', + ConditionExpression='remaining_credits >= :cost', ExpressionAttributeValues={':cost': cost, ':delta': -cost}, ) except botocore.exceptions.ClientError as e: From 3b078b888e506a1c59932cea47caec7658411526 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 11:27:43 -0900 Subject: [PATCH 013/115] decimals --- lib/dynamo/dynamo/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index bc54480a2..692c3f860 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -33,7 +33,7 @@ def decrement_credits(user_id: str, cost: float) -> None: Key={'user_id': user_id}, UpdateExpression='ADD remaining_credits :delta', ConditionExpression='remaining_credits >= :cost', - ExpressionAttributeValues={':cost': cost, ':delta': -cost}, + ExpressionAttributeValues={':cost': Decimal(cost), ':delta': Decimal(-cost)}, ) except botocore.exceptions.ClientError as e: if e.response['Error']['Code'] == 'ConditionalCheckFailedException': From ef6ecce34967de41ada81d95c8e21398c5a83671 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 11:31:39 -0900 Subject: [PATCH 014/115] pass user_id to decrement_credits --- lib/dynamo/dynamo/jobs.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index edb276e49..bee500a16 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -22,12 +22,12 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: table = DYNAMODB_RESOURCE.Table(environ['JOBS_TABLE_NAME']) request_time = format_time(datetime.now(timezone.utc)) - user = dynamo.user.get_user(user_id) - if not user: - user = dynamo.user.create_user(user_id) + user_record = dynamo.user.get_user(user_id) + if not user_record: + user_record = dynamo.user.create_user(user_id) - remaining_credits = user['remaining_credits'] - priority_override = user.get('priority_override') + remaining_credits = user_record['remaining_credits'] + priority_override = user_record.get('priority_override') prepared_jobs = [] for job in jobs: @@ -61,7 +61,7 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: for prepared_job in prepared_jobs: table.put_item(Item=convert_floats_to_decimals(prepared_job)) # TODO: handle "negative balance" ValueError, which indicates a race condition - dynamo.user.decrement_credits(user, total_cost) + dynamo.user.decrement_credits(user_record['user_id'], total_cost) return prepared_jobs From 7d81e73d0c4771e2f64a182cfbb213e8b0d3c05c Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 19 Jan 2024 11:41:05 -0900 Subject: [PATCH 015/115] test for dynamo.user.create_user --- lib/dynamo/dynamo/user.py | 1 - tests/test_dynamo/test_user.py | 29 ++++++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 692c3f860..fa2c36957 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -13,7 +13,6 @@ def get_user(user_id: str) -> Optional[dict]: return response.get('Item') -# TODO unit tests def create_user(user_id: str) -> dict: table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) if get_user(user_id): diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 1366e6c42..f10903ab3 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -1,16 +1,17 @@ +import pytest + import dynamo -# TODO update these field names? def test_get_user(tables): table_items = [ { 'user_id': 'user1', - 'max_jobs_per_user': 5 + 'remaining_credits': 5 }, { 'user_id': 'user2', - 'max_jobs_per_user': 15 + 'remaining_credits': 15 }, ] for item in table_items: @@ -19,3 +20,25 @@ def test_get_user(tables): assert dynamo.user.get_user('user1') == table_items[0] assert dynamo.user.get_user('user2') == table_items[1] assert dynamo.user.get_user('foo') is None + + +def test_create_user(tables, monkeypatch): + monkeypatch.setenv('MONTHLY_JOB_QUOTA_PER_USER', '25') + + user1 = dynamo.user.create_user('user1') + assert user1 == { + 'user_id': 'user1', + 'remaining_credits': 25, + } + + user2 = dynamo.user.create_user('user2') + assert user2 == { + 'user_id': 'user2', + 'remaining_credits': 25, + } + + response = tables.users_table.scan() + assert response['Items'] == [user1, user2] + + with pytest.raises(ValueError): + dynamo.user.create_user('user1') From 7a3e8c6c3f432d42a64a0623698b785520bcd6e5 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 11:56:36 -0900 Subject: [PATCH 016/115] fix test_put_jobs --- tests/test_dynamo/test_jobs.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index af227c09f..03f6654af 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -1,6 +1,5 @@ from datetime import datetime, timezone from decimal import Decimal -from unittest.mock import MagicMock, call, patch import pytest from conftest import list_have_same_elements @@ -182,7 +181,6 @@ def test_query_jobs_by_type(tables): assert list_have_same_elements(response, table_items[:2]) -# TODO update def test_put_jobs(tables): payload = [ { @@ -199,12 +197,13 @@ def test_put_jobs(tables): assert len(jobs) == 3 for job in jobs: assert set(job.keys()) == { - 'name', 'job_id', 'user_id', 'status_code', 'execution_started', 'request_time', 'priority' + 'name', 'job_id', 'user_id', 'status_code', 'execution_started', 'request_time', 'priority', 'credit_cost' } assert job['request_time'] <= dynamo.util.format_time(datetime.now(timezone.utc)) assert job['user_id'] == 'user1' assert job['status_code'] == 'PENDING' assert job['execution_started'] is False + assert job['credit_cost'] == 1.0 response = tables.jobs_table.scan() assert response['Items'] == jobs From 5426de40197b893de7fdbe89368cba4c3cab4fb4 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 19 Jan 2024 11:57:28 -0900 Subject: [PATCH 017/115] add test for dynamo.user.decrement_credits --- lib/dynamo/dynamo/user.py | 1 + tests/test_dynamo/test_user.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index fa2c36957..3e2bf6299 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -36,6 +36,7 @@ def decrement_credits(user_id: str, cost: float) -> None: ) except botocore.exceptions.ClientError as e: if e.response['Error']['Code'] == 'ConditionalCheckFailedException': + # TODO provide better error message accounting for case where user_id does not exist in the table raise ValueError( f'Subtracting cost {cost} from user\'s remaining credits would result in a negative balance' ) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index f10903ab3..4a9d9e04b 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -42,3 +42,32 @@ def test_create_user(tables, monkeypatch): with pytest.raises(ValueError): dynamo.user.create_user('user1') + + +def test_decrement_credits(tables): + with pytest.raises(ValueError): + dynamo.user.decrement_credits('foo', 1) + + user = {'user_id': 'foo', 'remaining_credits': 25} + tables.users_table.put_item(Item=user) + + with pytest.raises(ValueError): + dynamo.user.decrement_credits('foo', -1) + + dynamo.user.decrement_credits('foo', 1) + response = tables.users_table.scan() + assert response['Items'] == [{'user_id': 'foo', 'remaining_credits': 24}] + + dynamo.user.decrement_credits('foo', 4) + response = tables.users_table.scan() + assert response['Items'] == [{'user_id': 'foo', 'remaining_credits': 20}] + + # TODO should this case set remaining_credits to zero? + with pytest.raises(ValueError): + dynamo.user.decrement_credits('foo', 21) + response = tables.users_table.scan() + assert response['Items'] == [{'user_id': 'foo', 'remaining_credits': 20}] + + dynamo.user.decrement_credits('foo', 20) + response = tables.users_table.scan() + assert response['Items'] == [{'user_id': 'foo', 'remaining_credits': 0}] From 21b41bebc2f4f2c6f8de9f6b3567bd7910d0a849 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 12:12:19 -0900 Subject: [PATCH 018/115] rename MONTHLY_JOB_QUOTA_PER_USER to DEFAULT_CREDITS_PER_USER --- .github/actions/deploy-hyp3/action.yml | 6 +++--- .github/workflows/deploy-credits-sandbox.yml | 2 +- .github/workflows/deploy-daac.yml | 2 +- .github/workflows/deploy-enterprise-test.yml | 2 +- .github/workflows/deploy-enterprise.yml | 2 +- apps/api/api-cf.yml.j2 | 2 +- lib/dynamo/dynamo/user.py | 2 +- tests/cfg.env | 2 +- tests/test_api/test_get_user.py | 2 ++ tests/test_api/test_submit_job.py | 1 + tests/test_dynamo/test_user.py | 2 +- 11 files changed, 14 insertions(+), 11 deletions(-) diff --git a/.github/actions/deploy-hyp3/action.yml b/.github/actions/deploy-hyp3/action.yml index 946b9faa3..839864f7b 100644 --- a/.github/actions/deploy-hyp3/action.yml +++ b/.github/actions/deploy-hyp3/action.yml @@ -35,8 +35,8 @@ inputs: CLOUDFORMATION_ROLE_ARN: description: "The CloudFormation role to use for this deployment" required: true - MONTHLY_JOB_QUOTA_PER_USER: - description: "The default number of jobs any user with an Earthdata Login can run per month" + DEFAULT_CREDITS_PER_USER: + description: "The default number of credits given to a new user" required: true JOB_FILES: description: "Space seperated list of job spec YAMLs to include" @@ -119,7 +119,7 @@ runs: $CERTIFICATE_ARN \ $ORIGIN_ACCESS_IDENTITY_ID \ $DISTRIBUTION_URL \ - MonthlyJobQuotaPerUser='${{ inputs.MONTHLY_JOB_QUOTA_PER_USER }}' \ + MonthlyJobQuotaPerUser='${{ inputs.DEFAULT_CREDITS_PER_USER }}' \ DefaultMaxvCpus='${{ inputs.DEFAULT_MAX_VCPUS }}' \ ExpandedMaxvCpus='${{ inputs.EXPANDED_MAX_VCPUS }}' \ MonthlyBudget='${{ inputs.MONTHLY_BUDGET }}' \ diff --git a/.github/workflows/deploy-credits-sandbox.yml b/.github/workflows/deploy-credits-sandbox.yml index 72bc80285..033af07cb 100644 --- a/.github/workflows/deploy-credits-sandbox.yml +++ b/.github/workflows/deploy-credits-sandbox.yml @@ -61,7 +61,7 @@ jobs: SUBNET_IDS: ${{ secrets.SUBNET_IDS }} SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} - MONTHLY_JOB_QUOTA_PER_USER: ${{ matrix.quota }} + DEFAULT_CREDITS_PER_USER: ${{ matrix.quota }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} EXPANDED_MAX_VCPUS: ${{ matrix.expanded_max_vcpus }} diff --git a/.github/workflows/deploy-daac.yml b/.github/workflows/deploy-daac.yml index 691866a1b..727e888fb 100644 --- a/.github/workflows/deploy-daac.yml +++ b/.github/workflows/deploy-daac.yml @@ -86,7 +86,7 @@ jobs: SUBNET_IDS: ${{ secrets.SUBNET_IDS }} SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} - MONTHLY_JOB_QUOTA_PER_USER: ${{ matrix.quota }} + DEFAULT_CREDITS_PER_USER: ${{ matrix.quota }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} EXPANDED_MAX_VCPUS: ${{ matrix.expanded_max_vcpus }} diff --git a/.github/workflows/deploy-enterprise-test.yml b/.github/workflows/deploy-enterprise-test.yml index eb64d691f..810d029a7 100644 --- a/.github/workflows/deploy-enterprise-test.yml +++ b/.github/workflows/deploy-enterprise-test.yml @@ -69,7 +69,7 @@ jobs: SUBNET_IDS: ${{ secrets.SUBNET_IDS }} SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} - MONTHLY_JOB_QUOTA_PER_USER: ${{ matrix.quota }} + DEFAULT_CREDITS_PER_USER: ${{ matrix.quota }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} EXPANDED_MAX_VCPUS: ${{ matrix.expanded_max_vcpus }} diff --git a/.github/workflows/deploy-enterprise.yml b/.github/workflows/deploy-enterprise.yml index a9b18864d..74101e500 100644 --- a/.github/workflows/deploy-enterprise.yml +++ b/.github/workflows/deploy-enterprise.yml @@ -230,7 +230,7 @@ jobs: SUBNET_IDS: ${{ secrets.SUBNET_IDS }} SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} - MONTHLY_JOB_QUOTA_PER_USER: ${{ matrix.quota }} + DEFAULT_CREDITS_PER_USER: ${{ matrix.quota }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} EXPANDED_MAX_VCPUS: ${{ matrix.expanded_max_vcpus }} diff --git a/apps/api/api-cf.yml.j2 b/apps/api/api-cf.yml.j2 index db2cc819e..9bf403a56 100644 --- a/apps/api/api-cf.yml.j2 +++ b/apps/api/api-cf.yml.j2 @@ -179,7 +179,7 @@ Resources: USERS_TABLE_NAME: !Ref UsersTable AUTH_PUBLIC_KEY: !Ref AuthPublicKey AUTH_ALGORITHM: !Ref AuthAlgorithm - MONTHLY_JOB_QUOTA_PER_USER: !Ref MonthlyJobQuotaPerUser + DEFAULT_CREDITS_PER_USER: !Ref MonthlyJobQuotaPerUser SYSTEM_AVAILABLE: !Ref SystemAvailable Code: src/ Handler: hyp3_api.lambda_handler.handler diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index fa2c36957..cf1faa299 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -17,7 +17,7 @@ def create_user(user_id: str) -> dict: table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) if get_user(user_id): raise ValueError(f'user {user_id} already exists') - user = {'user_id': user_id, 'remaining_credits': Decimal(os.environ['MONTHLY_JOB_QUOTA_PER_USER'])} + user = {'user_id': user_id, 'remaining_credits': Decimal(os.environ['DEFAULT_CREDITS_PER_USER'])} table.put_item(Item=user) return user diff --git a/tests/cfg.env b/tests/cfg.env index ee8c72400..a01fd7e08 100644 --- a/tests/cfg.env +++ b/tests/cfg.env @@ -3,7 +3,7 @@ JOBS_TABLE_NAME=hyp3-db-table-job USERS_TABLE_NAME=hyp3-db-table-user AUTH_PUBLIC_KEY=123456789 AUTH_ALGORITHM=HS256 -MONTHLY_JOB_QUOTA_PER_USER=25 +DEFAULT_CREDITS_PER_USER=25 SYSTEM_AVAILABLE=true AWS_DEFAULT_REGION=us-west-2 AWS_ACCESS_KEY_ID=testing diff --git a/tests/test_api/test_get_user.py b/tests/test_api/test_get_user.py index c564dbc68..22b617cf8 100644 --- a/tests/test_api/test_get_user.py +++ b/tests/test_api/test_get_user.py @@ -6,6 +6,7 @@ from dynamo.util import format_time +# TODO update def test_get_user(client, tables, monkeypatch): monkeypatch.setenv('MONTHLY_JOB_QUOTA_PER_USER', '25') request_time = format_time(datetime.now(timezone.utc)) @@ -35,6 +36,7 @@ def test_get_user(client, tables, monkeypatch): } +# TODO update def test_user_at_quota(client, tables, monkeypatch): monkeypatch.setenv('MONTHLY_JOB_QUOTA_PER_USER', '25') request_time = format_time(datetime.now(timezone.utc)) diff --git a/tests/test_api/test_submit_job.py b/tests/test_api/test_submit_job.py index 9d7bf53df..02fdf3e59 100644 --- a/tests/test_api/test_submit_job.py +++ b/tests/test_api/test_submit_job.py @@ -112,6 +112,7 @@ def test_submit_many_jobs(client, tables): assert response.status_code == HTTPStatus.BAD_REQUEST +# TODO update def test_submit_exceeds_quota(client, tables, monkeypatch): login(client) time_for_previous_month = format_time(datetime.now(timezone.utc) - timedelta(days=32)) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index f10903ab3..808e5c9fc 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -23,7 +23,7 @@ def test_get_user(tables): def test_create_user(tables, monkeypatch): - monkeypatch.setenv('MONTHLY_JOB_QUOTA_PER_USER', '25') + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') user1 = dynamo.user.create_user('user1') assert user1 == { From 51652a66b843585e2d2536998a455946a3eea2da Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 19 Jan 2024 12:16:21 -0900 Subject: [PATCH 019/115] update test for api GET /user endpoint --- tests/test_api/test_get_user.py | 96 +++++++++++---------------------- 1 file changed, 32 insertions(+), 64 deletions(-) diff --git a/tests/test_api/test_get_user.py b/tests/test_api/test_get_user.py index c564dbc68..b5bb7eb86 100644 --- a/tests/test_api/test_get_user.py +++ b/tests/test_api/test_get_user.py @@ -6,88 +6,56 @@ from dynamo.util import format_time -def test_get_user(client, tables, monkeypatch): +def test_get_new_user(client, tables, monkeypatch): monkeypatch.setenv('MONTHLY_JOB_QUOTA_PER_USER', '25') - request_time = format_time(datetime.now(timezone.utc)) - user = 'user_with_jobs' - items = [ - make_db_record('job1', user_id=user, request_time=request_time, status_code='PENDING', name='job1'), - make_db_record('job2', user_id=user, request_time=request_time, status_code='RUNNING', name='job1'), - make_db_record('job3', user_id=user, request_time=request_time, status_code='FAILED', name='job2'), - make_db_record('job4', user_id=user, request_time=request_time, status_code='SUCCEEDED', name=None) - ] - for item in items: - tables.jobs_table.put_item(Item=item) - login(client, 'user_with_jobs') + login(client, 'user') response = client.get(USER_URI) assert response.status_code == HTTPStatus.OK assert response.json == { - 'user_id': 'user_with_jobs', - 'quota': { - 'max_jobs_per_month': 25, - 'remaining': 21, - }, - 'job_names': [ - 'job1', - 'job2', - ], + 'user_id': 'user', + 'remaining_credits': 25, + 'job_names': [], } -def test_user_at_quota(client, tables, monkeypatch): - monkeypatch.setenv('MONTHLY_JOB_QUOTA_PER_USER', '25') - request_time = format_time(datetime.now(timezone.utc)) - - items = [make_db_record(f'job{ii}', request_time=request_time) for ii in range(0, 24)] - for item in items: - tables.jobs_table.put_item(Item=item) - - login(client) - response = client.get(USER_URI) - assert response.status_code == HTTPStatus.OK - assert response.json['quota']['remaining'] == 1 - - tables.jobs_table.put_item(Item=make_db_record('anotherJob', request_time=request_time)) - response = client.get(USER_URI) - assert response.status_code == HTTPStatus.OK - assert response.json['quota']['remaining'] == 0 - - tables.jobs_table.put_item(Item=make_db_record('yetAnotherJob', request_time=request_time)) - response = client.get(USER_URI) - assert response.status_code == HTTPStatus.OK - assert response.json['quota']['remaining'] == 0 - - -def test_get_user_custom_quota(client, tables): - username = 'user_with_custom_quota' - login(client, username) - tables.users_table.put_item(Item={'user_id': username, 'max_jobs_per_month': 50}) +def test_get_existing_user(client, tables): + user = {'user_id': 'user', 'remaining_credits': None} + tables.users_table.put_item(Item=user) + login(client, 'user') response = client.get(USER_URI) assert response.status_code == HTTPStatus.OK assert response.json == { - 'user_id': username, - 'quota': { - 'max_jobs_per_month': 50, - 'remaining': 50, - }, + 'user_id': 'user', + 'remaining_credits': None, 'job_names': [], } -def test_get_user_no_quota(client, tables): - username = 'user_with_no_quota' - login(client, username) - tables.users_table.put_item(Item={'user_id': username, 'max_jobs_per_month': None}) +def test_get_user_with_jobs(client, tables): + user_id = 'user_with_jobs' + user = {'user_id': user_id, 'remaining_credits': 20, 'foo': 'bar'} + tables.users_table.put_item(Item=user) + + request_time = format_time(datetime.now(timezone.utc)) + items = [ + make_db_record('job1', user_id=user_id, request_time=request_time, status_code='PENDING', name='job1'), + make_db_record('job2', user_id=user_id, request_time=request_time, status_code='RUNNING', name='job1'), + make_db_record('job3', user_id=user_id, request_time=request_time, status_code='FAILED', name='job2'), + make_db_record('job4', user_id=user_id, request_time=request_time, status_code='SUCCEEDED', name=None) + ] + for item in items: + tables.jobs_table.put_item(Item=item) + login(client, 'user_with_jobs') response = client.get(USER_URI) assert response.status_code == HTTPStatus.OK assert response.json == { - 'user_id': username, - 'quota': { - 'max_jobs_per_month': None, - 'remaining': None, - }, - 'job_names': [], + 'user_id': 'user_with_jobs', + 'remaining_credits': 20, + 'job_names': [ + 'job1', + 'job2', + ], } From 2f8ffb2642aeb253fed5f7cbde5db3699d489ff9 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 13:52:53 -0900 Subject: [PATCH 020/115] rename MonthlyJobQuotaPerUser to DefaultCreditsPerUser --- .github/actions/deploy-hyp3/action.yml | 2 +- apps/api/api-cf.yml.j2 | 4 ++-- apps/main-cf.yml.j2 | 6 +++--- docs/deployments/ASF-deployment.md | 2 +- docs/deployments/JPL-deployment.md | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/actions/deploy-hyp3/action.yml b/.github/actions/deploy-hyp3/action.yml index 839864f7b..f5dbbc5ee 100644 --- a/.github/actions/deploy-hyp3/action.yml +++ b/.github/actions/deploy-hyp3/action.yml @@ -119,7 +119,7 @@ runs: $CERTIFICATE_ARN \ $ORIGIN_ACCESS_IDENTITY_ID \ $DISTRIBUTION_URL \ - MonthlyJobQuotaPerUser='${{ inputs.DEFAULT_CREDITS_PER_USER }}' \ + DefaultCreditsPerUser='${{ inputs.DEFAULT_CREDITS_PER_USER }}' \ DefaultMaxvCpus='${{ inputs.DEFAULT_MAX_VCPUS }}' \ ExpandedMaxvCpus='${{ inputs.EXPANDED_MAX_VCPUS }}' \ MonthlyBudget='${{ inputs.MONTHLY_BUDGET }}' \ diff --git a/apps/api/api-cf.yml.j2 b/apps/api/api-cf.yml.j2 index 9bf403a56..528a9fb9a 100644 --- a/apps/api/api-cf.yml.j2 +++ b/apps/api/api-cf.yml.j2 @@ -12,7 +12,7 @@ Parameters: AuthAlgorithm: Type: String - MonthlyJobQuotaPerUser: + DefaultCreditsPerUser: Type: Number SystemAvailable: @@ -179,7 +179,7 @@ Resources: USERS_TABLE_NAME: !Ref UsersTable AUTH_PUBLIC_KEY: !Ref AuthPublicKey AUTH_ALGORITHM: !Ref AuthAlgorithm - DEFAULT_CREDITS_PER_USER: !Ref MonthlyJobQuotaPerUser + DEFAULT_CREDITS_PER_USER: !Ref DefaultCreditsPerUser SYSTEM_AVAILABLE: !Ref SystemAvailable Code: src/ Handler: hyp3_api.lambda_handler.handler diff --git a/apps/main-cf.yml.j2 b/apps/main-cf.yml.j2 index ce1d9abbf..d16ab4211 100644 --- a/apps/main-cf.yml.j2 +++ b/apps/main-cf.yml.j2 @@ -31,8 +31,8 @@ Parameters: Type: String Default: RS256 - MonthlyJobQuotaPerUser: - Description: Number of jobs each user is allowed per month. + DefaultCreditsPerUser: + Description: The default number of credits given to a new user. Type: Number MinValue: 0 @@ -115,7 +115,7 @@ Resources: UsersTable: !Ref UsersTable AuthPublicKey: !Ref AuthPublicKey AuthAlgorithm: !Ref AuthAlgorithm - MonthlyJobQuotaPerUser: !Ref MonthlyJobQuotaPerUser + DefaultCreditsPerUser: !Ref DefaultCreditsPerUser SystemAvailable: !Ref SystemAvailable {% if security_environment == 'EDC' %} VpcId: !Ref VpcId diff --git a/docs/deployments/ASF-deployment.md b/docs/deployments/ASF-deployment.md index 5714ceac2..38c1a1489 100644 --- a/docs/deployments/ASF-deployment.md +++ b/docs/deployments/ASF-deployment.md @@ -93,5 +93,5 @@ aws cloudformation deploy \ DomainName= \ CertificateArn= \ SecretArn= \ - MonthlyJobQuotaPerUser=0 + DefaultCreditsPerUser=0 ``` diff --git a/docs/deployments/JPL-deployment.md b/docs/deployments/JPL-deployment.md index 341a95e56..45b9d25e2 100644 --- a/docs/deployments/JPL-deployment.md +++ b/docs/deployments/JPL-deployment.md @@ -93,7 +93,7 @@ aws cloudformation deploy \ DomainName= \ CertificateArn= \ SecretArn= \ - MonthlyJobQuotaPerUser=0 + DefaultCreditsPerUser=0 ``` ## 5. Post deployment From 683aa19c0463f822583fb4eda9d73ec07ca5f512 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 14:00:56 -0900 Subject: [PATCH 021/115] rename quota to default_credits_per_user in deploy matrix --- .github/workflows/deploy-credits-sandbox.yml | 4 +-- .github/workflows/deploy-daac.yml | 6 ++--- .github/workflows/deploy-enterprise-test.yml | 4 +-- .github/workflows/deploy-enterprise.yml | 26 ++++++++++---------- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/.github/workflows/deploy-credits-sandbox.yml b/.github/workflows/deploy-credits-sandbox.yml index 033af07cb..018d0df08 100644 --- a/.github/workflows/deploy-credits-sandbox.yml +++ b/.github/workflows/deploy-credits-sandbox.yml @@ -19,7 +19,7 @@ jobs: template_bucket: cf-templates-1hz9ldhhl4ahu-us-west-2 image_tag: test product_lifetime_in_days: 14 - quota: 0 + default_credits_per_user: 0 deploy_ref: refs/heads/credits-sandbox job_files: job_spec/AUTORIFT.yml job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml job_spec/INSAR_ISCE_BURST.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge @@ -61,7 +61,7 @@ jobs: SUBNET_IDS: ${{ secrets.SUBNET_IDS }} SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} - DEFAULT_CREDITS_PER_USER: ${{ matrix.quota }} + DEFAULT_CREDITS_PER_USER: ${{ matrix.default_credits_per_user }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} EXPANDED_MAX_VCPUS: ${{ matrix.expanded_max_vcpus }} diff --git a/.github/workflows/deploy-daac.yml b/.github/workflows/deploy-daac.yml index 727e888fb..bf15a74ee 100644 --- a/.github/workflows/deploy-daac.yml +++ b/.github/workflows/deploy-daac.yml @@ -21,7 +21,7 @@ jobs: template_bucket: cf-templates-118mtzosmrltk-us-west-2 image_tag: latest product_lifetime_in_days: 14 - quota: 1000 + default_credits_per_user: 1000 deploy_ref: refs/heads/main job_files: job_spec/AUTORIFT.yml job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml job_spec/INSAR_ISCE_BURST.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge @@ -38,7 +38,7 @@ jobs: template_bucket: cf-templates-118ylv0o6jp2n-us-west-2 image_tag: test product_lifetime_in_days: 14 - quota: 1000 + default_credits_per_user: 1000 deploy_ref: refs/heads/develop job_files: >- job_spec/AUTORIFT.yml @@ -86,7 +86,7 @@ jobs: SUBNET_IDS: ${{ secrets.SUBNET_IDS }} SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} - DEFAULT_CREDITS_PER_USER: ${{ matrix.quota }} + DEFAULT_CREDITS_PER_USER: ${{ matrix.default_credits_per_user }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} EXPANDED_MAX_VCPUS: ${{ matrix.expanded_max_vcpus }} diff --git a/.github/workflows/deploy-enterprise-test.yml b/.github/workflows/deploy-enterprise-test.yml index 810d029a7..f335aee20 100644 --- a/.github/workflows/deploy-enterprise-test.yml +++ b/.github/workflows/deploy-enterprise-test.yml @@ -19,7 +19,7 @@ jobs: template_bucket: cf-templates-1iw894v4yzqya-us-west-2 image_tag: test product_lifetime_in_days: 14 - quota: 0 + default_credits_per_user: 0 deploy_ref: refs/heads/develop job_files: >- job_spec/AUTORIFT_ITS_LIVE.yml @@ -69,7 +69,7 @@ jobs: SUBNET_IDS: ${{ secrets.SUBNET_IDS }} SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} - DEFAULT_CREDITS_PER_USER: ${{ matrix.quota }} + DEFAULT_CREDITS_PER_USER: ${{ matrix.default_credits_per_user }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} EXPANDED_MAX_VCPUS: ${{ matrix.expanded_max_vcpus }} diff --git a/.github/workflows/deploy-enterprise.yml b/.github/workflows/deploy-enterprise.yml index 74101e500..9167d37be 100644 --- a/.github/workflows/deploy-enterprise.yml +++ b/.github/workflows/deploy-enterprise.yml @@ -19,7 +19,7 @@ jobs: template_bucket: cf-templates-3o5lnspmwmzg-us-west-2 image_tag: latest product_lifetime_in_days: 45 - quota: 0 + default_credits_per_user: 0 job_files: >- job_spec/AUTORIFT_ITS_LIVE.yml job_spec/AUTORIFT_ITS_LIVE_TEST.yml @@ -37,7 +37,7 @@ jobs: template_bucket: cf-templates-v4pvone059de-us-west-2 image_tag: latest product_lifetime_in_days: 180 - quota: 0 + default_credits_per_user: 0 job_files: job_spec/INSAR_ISCE.yml job_spec/INSAR_ISCE_TEST.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge default_max_vcpus: 10000 @@ -52,7 +52,7 @@ jobs: template_bucket: cf-templates-1or0efwqffkgd-us-west-2 image_tag: latest product_lifetime_in_days: 60 - quota: 0 + default_credits_per_user: 0 job_files: job_spec/INSAR_ISCE.yml job_spec/INSAR_ISCE_TEST.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge default_max_vcpus: 0 @@ -67,7 +67,7 @@ jobs: template_bucket: cf-templates-gdeyr9hh8rzs-us-west-2 image_tag: latest product_lifetime_in_days: 14 - quota: 0 + default_credits_per_user: 0 job_files: job_spec/INSAR_ISCE.yml job_spec/INSAR_ISCE_TEST.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge default_max_vcpus: 1600 @@ -82,7 +82,7 @@ jobs: template_bucket: cf-templates-1x4a21iq1cba7-us-west-2 image_tag: latest product_lifetime_in_days: 365000 - quota: 0 + default_credits_per_user: 0 job_files: job_spec/INSAR_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -97,7 +97,7 @@ jobs: template_bucket: cf-templates-1217di08q7vwl-us-west-2 image_tag: latest product_lifetime_in_days: 14 - quota: 0 + default_credits_per_user: 0 job_files: job_spec/RTC_GAMMA.yml job_spec/WATER_MAP.yml job_spec/WATER_MAP_EQ.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -112,7 +112,7 @@ jobs: template_bucket: cf-templates-15gmiot9prm67-us-west-2 image_tag: latest product_lifetime_in_days: 90 - quota: 0 + default_credits_per_user: 0 job_files: job_spec/RTC_GAMMA.yml job_spec/WATER_MAP.yml job_spec/WATER_MAP_EQ.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 1600 @@ -127,7 +127,7 @@ jobs: template_bucket: cf-templates-xlga17noink6-us-west-2 image_tag: latest product_lifetime_in_days: 30 - quota: 0 + default_credits_per_user: 0 job_files: job_spec/INSAR_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -142,7 +142,7 @@ jobs: template_bucket: cf-templates-j4kd746vpsuv-us-east-1 image_tag: latest product_lifetime_in_days: 14 - quota: 0 + default_credits_per_user: 0 job_files: job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -157,7 +157,7 @@ jobs: template_bucket: cf-templates-ez0805f6vy20-us-west-2 image_tag: latest product_lifetime_in_days: 14 - quota: 0 + default_credits_per_user: 0 job_files: job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -172,7 +172,7 @@ jobs: template_bucket: cf-templates-1qx2mwia5g4kh-us-west-2 image_tag: latest product_lifetime_in_days: 30 - quota: 0 + default_credits_per_user: 0 job_files: job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -189,7 +189,7 @@ jobs: # TODO product lifetime could be much shorter since they all get transferred to a separate # S3 bucket, but maybe we want to allow for a backlog of products-to-be-transferred? product_lifetime_in_days: 14 - quota: 0 + default_credits_per_user: 0 job_files: job_spec/WATER_MAP.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -230,7 +230,7 @@ jobs: SUBNET_IDS: ${{ secrets.SUBNET_IDS }} SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} - DEFAULT_CREDITS_PER_USER: ${{ matrix.quota }} + DEFAULT_CREDITS_PER_USER: ${{ matrix.default_credits_per_user }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} EXPANDED_MAX_VCPUS: ${{ matrix.expanded_max_vcpus }} From 1fc01910d1abcbbf8ae003370a5291337e32d6f0 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 14:22:47 -0900 Subject: [PATCH 022/115] improve docstring --- lib/dynamo/dynamo/jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index bee500a16..ce79b54b3 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -10,7 +10,7 @@ class InsufficientCreditsError(Exception): - """Raised when trying to submit more jobs that user has remaining""" + """Raised when trying to submit jobs whose total cost exceeds the user's remaining credits.""" def get_credit_cost(job: dict) -> float: From 6b8f4b62c186ec13a22f4f19c3087ad98e5506a4 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 15:13:49 -0900 Subject: [PATCH 023/115] refactor put_jobs --- lib/dynamo/dynamo/jobs.py | 66 ++++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 22 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index ce79b54b3..d066ae10a 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -1,6 +1,6 @@ from datetime import datetime, timezone from os import environ -from typing import List +from typing import List, Optional from uuid import uuid4 from boto3.dynamodb.conditions import Attr, Key @@ -29,29 +29,21 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: remaining_credits = user_record['remaining_credits'] priority_override = user_record.get('priority_override') + last_job_priority = None prepared_jobs = [] for job in jobs: - prepared_job = { - 'job_id': str(uuid4()), - 'user_id': user_id, - 'status_code': 'PENDING', - 'execution_started': False, - 'request_time': request_time, - 'credit_cost': get_credit_cost(job), - **job, - } - if priority_override: - priority = priority_override - elif remaining_credits is None: - priority = 0 - elif prepared_jobs: - priority = max(prepared_jobs[-1]['priority'] - int(prepared_job['credit_cost']), 0) - else: - priority = min(int(remaining_credits), 9999) - prepared_job['priority'] = priority + prepared_job = prepare_job_for_database( + job=job, + user_id=user_id, + request_time=request_time, + remaining_credits=remaining_credits, + priority_override=priority_override, + last_job_priority=last_job_priority, + ) prepared_jobs.append(prepared_job) + last_job_priority = prepared_job['priority'] - total_cost = sum([job['credit_cost'] for job in prepared_jobs]) + total_cost = sum(job['credit_cost'] for job in prepared_jobs) if remaining_credits is not None and total_cost > remaining_credits: raise InsufficientCreditsError( f'These jobs would cost {total_cost} credits, but you have only {remaining_credits} remaining.' @@ -60,11 +52,41 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: if not dry_run: for prepared_job in prepared_jobs: table.put_item(Item=convert_floats_to_decimals(prepared_job)) - # TODO: handle "negative balance" ValueError, which indicates a race condition - dynamo.user.decrement_credits(user_record['user_id'], total_cost) + # TODO: handle race condition ValueError from decrement_credits + dynamo.user.decrement_credits(user_id, total_cost) + return prepared_jobs +def prepare_job_for_database( + job: dict, + user_id: str, + request_time: str, + remaining_credits: Optional[float], + priority_override: Optional[int], + last_job_priority: Optional[int], +) -> dict: + credit_cost = get_credit_cost(job) + if priority_override: + priority = priority_override + elif remaining_credits is None: + priority = 0 + elif last_job_priority is None: + priority = min(int(remaining_credits), 9999) + else: + priority = max(last_job_priority - int(credit_cost), 0) + return { + 'job_id': str(uuid4()), + 'user_id': user_id, + 'status_code': 'PENDING', + 'execution_started': False, + 'request_time': request_time, + 'credit_cost': credit_cost, + 'priority': priority, + **job, + } + + def query_jobs(user, start=None, end=None, status_code=None, name=None, job_type=None, start_key=None): table = DYNAMODB_RESOURCE.Table(environ['JOBS_TABLE_NAME']) From 1ad2d8d0804affb342b62b445798ed99be19f042 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 15:31:05 -0900 Subject: [PATCH 024/115] improve error message in decrement_credits --- lib/dynamo/dynamo/jobs.py | 2 +- lib/dynamo/dynamo/user.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index d066ae10a..734d8bc47 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -52,7 +52,7 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: if not dry_run: for prepared_job in prepared_jobs: table.put_item(Item=convert_floats_to_decimals(prepared_job)) - # TODO: handle race condition ValueError from decrement_credits + # TODO: handle ValueError from decrement_credits? or just let it propagate? dynamo.user.decrement_credits(user_id, total_cost) return prepared_jobs diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index f8dc7614e..03a302cdb 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -36,8 +36,7 @@ def decrement_credits(user_id: str, cost: float) -> None: ) except botocore.exceptions.ClientError as e: if e.response['Error']['Code'] == 'ConditionalCheckFailedException': - # TODO provide better error message accounting for case where user_id does not exist in the table raise ValueError( - f'Subtracting cost {cost} from user\'s remaining credits would result in a negative balance' + f"User '{user_id}' does not exist or they have fewer than {cost} credits" ) raise From 20fffb2429c11641cb7180bcee1d69ad6ca8b015 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 15:33:18 -0900 Subject: [PATCH 025/115] add TODO --- tests/test_dynamo/test_jobs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 03f6654af..25d664fdb 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -181,6 +181,7 @@ def test_query_jobs_by_type(tables): assert list_have_same_elements(response, table_items[:2]) +# TODO update? def test_put_jobs(tables): payload = [ { From 34d0edaa93510d22d1be23ffb4ea35a4874937b1 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 16:06:04 -0900 Subject: [PATCH 026/115] add TODO --- tests/test_api/test_submit_job.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_api/test_submit_job.py b/tests/test_api/test_submit_job.py index 02fdf3e59..179b166b7 100644 --- a/tests/test_api/test_submit_job.py +++ b/tests/test_api/test_submit_job.py @@ -6,6 +6,8 @@ from dynamo.util import format_time +# TODO update for credits system? + def test_submit_one_job(client, tables): login(client) batch = [make_job()] From af4b76f2283b61bd7b790843c31c111b6c250cb2 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 16:19:53 -0900 Subject: [PATCH 027/115] update test_put_jobs --- tests/test_dynamo/test_jobs.py | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 25d664fdb..e2f0ba944 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -1,3 +1,4 @@ +import os from datetime import datetime, timezone from decimal import Decimal @@ -181,20 +182,10 @@ def test_query_jobs_by_type(tables): assert list_have_same_elements(response, table_items[:2]) -# TODO update? def test_put_jobs(tables): - payload = [ - { - 'name': 'name1', - }, - { - 'name': 'name1', - }, - { - 'name': 'name2', - }, - ] + payload = [{'name': 'name1'}, {'name': 'name1'}, {'name': 'name2'}] jobs = dynamo.jobs.put_jobs('user1', payload) + assert len(jobs) == 3 for job in jobs: assert set(job.keys()) == { @@ -204,10 +195,14 @@ def test_put_jobs(tables): assert job['user_id'] == 'user1' assert job['status_code'] == 'PENDING' assert job['execution_started'] is False - assert job['credit_cost'] == 1.0 + assert job['credit_cost'] == 1 - response = tables.jobs_table.scan() - assert response['Items'] == jobs + jobs_response = tables.jobs_table.scan() + assert jobs_response['Items'] == jobs + + expected_remaining_credits = int(os.environ['DEFAULT_CREDITS_PER_USER']) - 3 + users_response = tables.users_table.scan() + assert users_response['Items'] == [{'user_id': 'user1', 'remaining_credits': expected_remaining_credits}] # TODO update From dfb84e9cea4613618dbe2e36bd5cacb559da3101 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 16:36:21 -0900 Subject: [PATCH 028/115] split test_put_jobs_no_quota into two new tests --- lib/dynamo/dynamo/user.py | 1 + tests/test_dynamo/test_jobs.py | 17 +++++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 03a302cdb..ee887339f 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -31,6 +31,7 @@ def decrement_credits(user_id: str, cost: float) -> None: table.update_item( Key={'user_id': user_id}, UpdateExpression='ADD remaining_credits :delta', + # TODO condition fails if remaining_credits is null ConditionExpression='remaining_credits >= :cost', ExpressionAttributeValues={':cost': Decimal(cost), ':delta': Decimal(-cost)}, ) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index e2f0ba944..368901e3d 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -205,15 +205,20 @@ def test_put_jobs(tables): assert users_response['Items'] == [{'user_id': 'user1', 'remaining_credits': expected_remaining_credits}] -# TODO update -def test_put_jobs_no_quota(tables, monkeypatch): - monkeypatch.setenv('MONTHLY_JOB_QUOTA_PER_USER', '1') +def test_put_jobs_insufficient_credits(tables, monkeypatch): + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '1') payload = [{'name': 'name1'}, {'name': 'name2'}] - with pytest.raises(dynamo.jobs.QuotaError): - jobs = dynamo.jobs.put_jobs('user1', payload) + with pytest.raises(dynamo.jobs.InsufficientCreditsError): + dynamo.jobs.put_jobs('user1', payload) + + +# TODO test fails because of bug in decrement_credits +def test_put_jobs_infinite_credits(tables, monkeypatch): + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '1') + payload = [{'name': 'name1'}, {'name': 'name2'}] - tables.users_table.put_item(Item={'user_id': 'user1', 'max_jobs_per_month': None}) + tables.users_table.put_item(Item={'user_id': 'user1', 'remaining_credits': None}) jobs = dynamo.jobs.put_jobs('user1', payload) From eb036670738257676073a58d56f7035d13bf521f Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 16:56:27 -0900 Subject: [PATCH 029/115] further improve error message in decrement_credits --- lib/dynamo/dynamo/user.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index ee887339f..31ddc96f6 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -38,6 +38,8 @@ def decrement_credits(user_id: str, cost: float) -> None: except botocore.exceptions.ClientError as e: if e.response['Error']['Code'] == 'ConditionalCheckFailedException': raise ValueError( - f"User '{user_id}' does not exist or they have fewer than {cost} credits" + f"Unable to update remaining_credits for user_id '{user_id}'." + f' It is likely that the user record does not exist, remaining_credits < {cost},' + ' or remaining_credits is of the wrong data type.' ) raise From e9a2a42125ada57a0309d1cb6fb2f6a6d091f728 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 16:58:42 -0900 Subject: [PATCH 030/115] do not attempt to decrement credits if remaining_credits is null --- lib/dynamo/dynamo/jobs.py | 3 ++- lib/dynamo/dynamo/user.py | 1 - tests/test_dynamo/test_jobs.py | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 734d8bc47..bc2a5e837 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -53,7 +53,8 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: for prepared_job in prepared_jobs: table.put_item(Item=convert_floats_to_decimals(prepared_job)) # TODO: handle ValueError from decrement_credits? or just let it propagate? - dynamo.user.decrement_credits(user_id, total_cost) + if remaining_credits is not None: + dynamo.user.decrement_credits(user_id, total_cost) return prepared_jobs diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 31ddc96f6..6278e0d2e 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -31,7 +31,6 @@ def decrement_credits(user_id: str, cost: float) -> None: table.update_item( Key={'user_id': user_id}, UpdateExpression='ADD remaining_credits :delta', - # TODO condition fails if remaining_credits is null ConditionExpression='remaining_credits >= :cost', ExpressionAttributeValues={':cost': Decimal(cost), ':delta': Decimal(-cost)}, ) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 368901e3d..015dca9ad 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -213,7 +213,6 @@ def test_put_jobs_insufficient_credits(tables, monkeypatch): dynamo.jobs.put_jobs('user1', payload) -# TODO test fails because of bug in decrement_credits def test_put_jobs_infinite_credits(tables, monkeypatch): monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '1') payload = [{'name': 'name1'}, {'name': 'name2'}] From 9d2bb6b7efaba2bc337a01a67b4d590736447ae3 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 16:59:53 -0900 Subject: [PATCH 031/115] move TODO --- lib/dynamo/dynamo/jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index bc2a5e837..9dcea1b11 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -52,8 +52,8 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: if not dry_run: for prepared_job in prepared_jobs: table.put_item(Item=convert_floats_to_decimals(prepared_job)) - # TODO: handle ValueError from decrement_credits? or just let it propagate? if remaining_credits is not None: + # TODO: handle ValueError from decrement_credits? or just let it propagate? dynamo.user.decrement_credits(user_id, total_cost) return prepared_jobs From 84ff3288a1acb0234fc41e14054a613e97e978a5 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 17:10:23 -0900 Subject: [PATCH 032/115] update test_put_jobs_priority_override --- tests/test_dynamo/test_jobs.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 015dca9ad..1f6216c7e 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -226,10 +226,9 @@ def test_put_jobs_infinite_credits(tables, monkeypatch): assert job['priority'] == 0 -# TODO update def test_put_jobs_priority_override(tables): payload = [{'name': 'name1'}, {'name': 'name2'}] - tables.users_table.put_item(Item={'user_id': 'user1', 'priority': 100}) + tables.users_table.put_item(Item={'user_id': 'user1', 'priority_override': 100, 'remaining_credits': 3}) jobs = dynamo.jobs.put_jobs('user1', payload) @@ -237,7 +236,7 @@ def test_put_jobs_priority_override(tables): for job in jobs: assert job['priority'] == 100 - tables.users_table.put_item(Item={'user_id': 'user1', 'priority': 550}) + tables.users_table.put_item(Item={'user_id': 'user1', 'priority_override': 550, 'remaining_credits': None}) jobs = dynamo.jobs.put_jobs('user1', payload) From f350492678e67b8eebe7299f9884596234b5cb97 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 17:26:20 -0900 Subject: [PATCH 033/115] update test_put_jobs_priority --- tests/test_dynamo/test_jobs.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 1f6216c7e..6f73d94c9 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -245,16 +245,27 @@ def test_put_jobs_priority_override(tables): assert job['priority'] == 550 -# TODO update def test_put_jobs_priority(tables): + tables.users_table.put_item(Item={'user_id': 'user1', 'remaining_credits': 10_001}) + jobs = [] jobs.extend(dynamo.jobs.put_jobs('user1', [{}])) - jobs.extend(dynamo.jobs.put_jobs('user1', [{}, {}])) - jobs.extend(dynamo.jobs.put_jobs('user2', [{}])) + jobs.extend(dynamo.jobs.put_jobs('user1', [{}])) + jobs.extend(dynamo.jobs.put_jobs('user1', [{}])) + jobs.extend(dynamo.jobs.put_jobs('user1', [{}])) + jobs.extend(dynamo.jobs.put_jobs('user1', [{}, {}, {}])) + assert jobs[0]['priority'] == 9999 - assert jobs[1]['priority'] == 9998 - assert jobs[2]['priority'] == 9997 - assert jobs[3]['priority'] == 9999 + assert jobs[1]['priority'] == 9999 + assert jobs[2]['priority'] == 9999 + assert jobs[3]['priority'] == 9998 + assert jobs[4]['priority'] == 9997 + assert jobs[5]['priority'] == 9996 + assert jobs[6]['priority'] == 9995 + + jobs.extend(dynamo.jobs.put_jobs('user2', [{}])) + + assert jobs[7]['priority'] == int(os.environ['DEFAULT_CREDITS_PER_USER']) # TODO update From f06b3a8d0491b293936745f0d19172fda95f4f07 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 19 Jan 2024 17:37:11 -0900 Subject: [PATCH 034/115] rename variable, add some TODOs --- lib/dynamo/dynamo/jobs.py | 14 ++++++++------ tests/test_dynamo/test_jobs.py | 9 ++++----- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 9dcea1b11..901f003d5 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -29,7 +29,7 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: remaining_credits = user_record['remaining_credits'] priority_override = user_record.get('priority_override') - last_job_priority = None + previous_job_priority = None prepared_jobs = [] for job in jobs: prepared_job = prepare_job_for_database( @@ -38,10 +38,10 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: request_time=request_time, remaining_credits=remaining_credits, priority_override=priority_override, - last_job_priority=last_job_priority, + previous_job_priority=previous_job_priority, ) prepared_jobs.append(prepared_job) - last_job_priority = prepared_job['priority'] + previous_job_priority = prepared_job['priority'] total_cost = sum(job['credit_cost'] for job in prepared_jobs) if remaining_credits is not None and total_cost > remaining_credits: @@ -65,17 +65,19 @@ def prepare_job_for_database( request_time: str, remaining_credits: Optional[float], priority_override: Optional[int], - last_job_priority: Optional[int], + previous_job_priority: Optional[int], ) -> dict: credit_cost = get_credit_cost(job) if priority_override: priority = priority_override elif remaining_credits is None: priority = 0 - elif last_job_priority is None: + # TODO if you have > 9999 credits, you can get higher job priority by submitting jobs one at a time; + # are we OK with this behavior? + elif previous_job_priority is None: priority = min(int(remaining_credits), 9999) else: - priority = max(last_job_priority - int(credit_cost), 0) + priority = max(previous_job_priority - int(credit_cost), 0) return { 'job_id': str(uuid4()), 'user_id': user_id, diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 6f73d94c9..9564146fe 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -249,15 +249,14 @@ def test_put_jobs_priority(tables): tables.users_table.put_item(Item={'user_id': 'user1', 'remaining_credits': 10_001}) jobs = [] - jobs.extend(dynamo.jobs.put_jobs('user1', [{}])) - jobs.extend(dynamo.jobs.put_jobs('user1', [{}])) - jobs.extend(dynamo.jobs.put_jobs('user1', [{}])) + jobs.extend(dynamo.jobs.put_jobs('user1', [{}, {}, {}])) jobs.extend(dynamo.jobs.put_jobs('user1', [{}])) jobs.extend(dynamo.jobs.put_jobs('user1', [{}, {}, {}])) + # TODO is this the behavior we want? assert jobs[0]['priority'] == 9999 - assert jobs[1]['priority'] == 9999 - assert jobs[2]['priority'] == 9999 + assert jobs[1]['priority'] == 9998 + assert jobs[2]['priority'] == 9997 assert jobs[3]['priority'] == 9998 assert jobs[4]['priority'] == 9997 assert jobs[5]['priority'] == 9996 From 1e681ed823f10c60f58b4cb8559d2e146c53855e Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 22 Jan 2024 09:43:41 -0900 Subject: [PATCH 035/115] fix test_put_jobs_priority_overflow --- tests/test_dynamo/test_jobs.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 9564146fe..8ce5589b5 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -267,10 +267,9 @@ def test_put_jobs_priority(tables): assert jobs[7]['priority'] == int(os.environ['DEFAULT_CREDITS_PER_USER']) -# TODO update def test_put_jobs_priority_overflow(tables, monkeypatch): - monkeypatch.setenv('MONTHLY_JOB_QUOTA_PER_USER', '10001') - many_jobs = [{} for ii in range(10001)] + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '10_001') + many_jobs = [{} for _ in range(10_001)] jobs = dynamo.jobs.put_jobs('user3', many_jobs) assert jobs[-1]['priority'] == 0 assert jobs[-2]['priority'] == 0 From d05769b9ff63f3ffb2959df6f80ae87f8aa7cdac Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 22 Jan 2024 10:02:11 -0900 Subject: [PATCH 036/115] fix test_submit_exceeds_quota --- tests/test_api/test_submit_job.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/tests/test_api/test_submit_job.py b/tests/test_api/test_submit_job.py index 179b166b7..586c3dc9e 100644 --- a/tests/test_api/test_submit_job.py +++ b/tests/test_api/test_submit_job.py @@ -114,25 +114,23 @@ def test_submit_many_jobs(client, tables): assert response.status_code == HTTPStatus.BAD_REQUEST -# TODO update def test_submit_exceeds_quota(client, tables, monkeypatch): login(client) - time_for_previous_month = format_time(datetime.now(timezone.utc) - timedelta(days=32)) - job_from_previous_month = make_db_record('0ddaeb98-7636-494d-9496-03ea4a7df266', - request_time=time_for_previous_month) - tables.jobs_table.put_item(Item=job_from_previous_month) + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') - monkeypatch.setenv('MONTHLY_JOB_QUOTA_PER_USER', '25') - batch = [make_job() for ii in range(25)] - setup_requests_mock(batch) + batch1 = [make_job() for _ in range(20)] + setup_requests_mock(batch1) - response = submit_batch(client, batch) - assert response.status_code == HTTPStatus.OK + response1 = submit_batch(client, batch1) + assert response1.status_code == HTTPStatus.OK - response = submit_batch(client) - assert response.status_code == HTTPStatus.BAD_REQUEST - assert '25 jobs' in response.json['detail'] - assert '0 jobs' in response.json['detail'] + batch2 = [make_job() for _ in range(10)] + setup_requests_mock(batch2) + + response2 = submit_batch(client, batch2) + assert response2.status_code == HTTPStatus.BAD_REQUEST + # TODO maybe we should fix dynamo.jobs.put_jobs so that the formatting (float vs. int) is consistent + assert response2.json['detail'] == 'These jobs would cost 10.0 credits, but you have only 5 remaining.' def test_submit_without_jobs(client): From 33e5c7a1f789599631a3030d613ac017a8aca3d5 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Mon, 22 Jan 2024 10:16:24 -0900 Subject: [PATCH 037/115] fix flake8 findings for imports --- lib/dynamo/dynamo/user.py | 1 + tests/test_api/test_submit_job.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 6278e0d2e..afd248076 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -4,6 +4,7 @@ from typing import Optional import botocore.exceptions + from dynamo.util import DYNAMODB_RESOURCE diff --git a/tests/test_api/test_submit_job.py b/tests/test_api/test_submit_job.py index 586c3dc9e..84a78f2b9 100644 --- a/tests/test_api/test_submit_job.py +++ b/tests/test_api/test_submit_job.py @@ -1,7 +1,7 @@ -from datetime import datetime, timedelta, timezone +from datetime import datetime, timezone from http import HTTPStatus -from test_api.conftest import DEFAULT_USERNAME, login, make_db_record, make_job, setup_requests_mock, submit_batch +from test_api.conftest import DEFAULT_USERNAME, login, make_job, setup_requests_mock, submit_batch from dynamo.util import format_time From 496944f3a54c176fc8dc98ab3125226aad8d6a8f Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Mon, 22 Jan 2024 10:35:15 -0900 Subject: [PATCH 038/115] Update apps/api/src/hyp3_api/handlers.py Co-authored-by: Jake Herrmann --- apps/api/src/hyp3_api/handlers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 7b7ee384c..8f770b3f4 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -79,6 +79,7 @@ def get_names_for_user(user): def get_user(user): + # TODO: maybe create a helper function for this since we do the same thing in `put_jobs` user_record = dynamo.user.get_user(user) if not user_record: user_record = dynamo.user.create_user(user) From aa58548344384ea62ffd8c327dd18b846aeb3dae Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Mon, 22 Jan 2024 10:35:27 -0900 Subject: [PATCH 039/115] Update apps/api/src/hyp3_api/handlers.py Co-authored-by: Jake Herrmann --- apps/api/src/hyp3_api/handlers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 8f770b3f4..d98bc29f9 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -87,5 +87,6 @@ def get_user(user): return { 'user_id': user, 'remaining_credits': user_record['remaining_credits'], + # TODO: count this as jobs are submitted, not every time `/user` is queried 'job_names': get_names_for_user(user) } From a206464d5a63518b4b802b771dac5616e0bc2173 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 22 Jan 2024 10:38:49 -0900 Subject: [PATCH 040/115] Update tests/test_api/test_submit_job.py --- tests/test_api/test_submit_job.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_api/test_submit_job.py b/tests/test_api/test_submit_job.py index 84a78f2b9..3e3cea044 100644 --- a/tests/test_api/test_submit_job.py +++ b/tests/test_api/test_submit_job.py @@ -6,8 +6,6 @@ from dynamo.util import format_time -# TODO update for credits system? - def test_submit_one_job(client, tables): login(client) batch = [make_job()] From de77a71b813d308e519016011254d08546fcadcd Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Mon, 22 Jan 2024 10:55:14 -0900 Subject: [PATCH 041/115] give api lambda dynamodb:PutItem permissions on user table --- apps/api/api-cf.yml.j2 | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/api/api-cf.yml.j2 b/apps/api/api-cf.yml.j2 index 528a9fb9a..5c3e38260 100644 --- a/apps/api/api-cf.yml.j2 +++ b/apps/api/api-cf.yml.j2 @@ -168,6 +168,7 @@ Resources: - Effect: Allow Action: - dynamodb:GetItem + - dynamodb:PutItem Resource: !Sub "arn:aws:dynamodb:${AWS::Region}:${AWS::AccountId}:table/${UsersTable}*" Lambda: From a23f33046ceea293ebfce8658d0da89d05c46f98 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Mon, 22 Jan 2024 11:07:42 -0900 Subject: [PATCH 042/115] add dynamodb:UpdateItem permissions on user table for API lambda --- apps/api/api-cf.yml.j2 | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/api/api-cf.yml.j2 b/apps/api/api-cf.yml.j2 index 5c3e38260..eabc8941d 100644 --- a/apps/api/api-cf.yml.j2 +++ b/apps/api/api-cf.yml.j2 @@ -169,6 +169,7 @@ Resources: Action: - dynamodb:GetItem - dynamodb:PutItem + - dynamodb:UpdateItem Resource: !Sub "arn:aws:dynamodb:${AWS::Region}:${AWS::AccountId}:table/${UsersTable}*" Lambda: From 6a332d5aaf6fffc8ab9bf7f5f1835200a5d989c2 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Mon, 22 Jan 2024 13:47:13 -0900 Subject: [PATCH 043/115] leverage batch writer in dynamo.jobs.put_jobs --- apps/api/api-cf.yml.j2 | 2 +- lib/dynamo/dynamo/jobs.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/api/api-cf.yml.j2 b/apps/api/api-cf.yml.j2 index eabc8941d..0fdd8525c 100644 --- a/apps/api/api-cf.yml.j2 +++ b/apps/api/api-cf.yml.j2 @@ -161,7 +161,7 @@ Resources: Resource: !Sub "arn:aws:logs:${AWS::Region}:${AWS::AccountId}:log-group:/aws/lambda/*" - Effect: Allow Action: - - dynamodb:PutItem + - dynamodb:BatchWriteItem - dynamodb:Query - dynamodb:GetItem Resource: !Sub "arn:aws:dynamodb:${AWS::Region}:${AWS::AccountId}:table/${JobsTable}*" diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 901f003d5..2b0f957a1 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -50,8 +50,9 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: ) if not dry_run: - for prepared_job in prepared_jobs: - table.put_item(Item=convert_floats_to_decimals(prepared_job)) + with table.batch_writer() as batch: + for prepared_job in prepared_jobs: + batch.put_item(Item=convert_floats_to_decimals(prepared_job)) if remaining_credits is not None: # TODO: handle ValueError from decrement_credits? or just let it propagate? dynamo.user.decrement_credits(user_id, total_cost) From c08a7cb4bc340c2e2e1bf370dba7453310f7ae70 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 22 Jan 2024 14:09:49 -0900 Subject: [PATCH 044/115] add tests for desired priority behavior --- lib/dynamo/dynamo/jobs.py | 1 + tests/test_dynamo/test_jobs.py | 38 +++++++++++++++++++++------------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 901f003d5..9fbd89324 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -77,6 +77,7 @@ def prepare_job_for_database( elif previous_job_priority is None: priority = min(int(remaining_credits), 9999) else: + # TODO should we raise exception if reach 0? priority = max(previous_job_priority - int(credit_cost), 0) return { 'job_id': str(uuid4()), diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 8ce5589b5..5721034ad 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -246,27 +246,37 @@ def test_put_jobs_priority_override(tables): def test_put_jobs_priority(tables): - tables.users_table.put_item(Item={'user_id': 'user1', 'remaining_credits': 10_001}) + tables.users_table.put_item(Item={'user_id': 'user1', 'remaining_credits': 30}) - jobs = [] - jobs.extend(dynamo.jobs.put_jobs('user1', [{}, {}, {}])) - jobs.extend(dynamo.jobs.put_jobs('user1', [{}])) - jobs.extend(dynamo.jobs.put_jobs('user1', [{}, {}, {}])) + jobs = dynamo.jobs.put_jobs(user_id='user1', jobs=[{}, {}, {}]) + assert jobs[0]['priority'] == 30 + assert jobs[1]['priority'] == 29 + assert jobs[2]['priority'] == 28 - # TODO is this the behavior we want? + jobs.extend(dynamo.jobs.put_jobs(user_id='user1', jobs=[{}, {}])) + assert jobs[3]['priority'] == 27 + assert jobs[4]['priority'] == 26 + + +def test_put_jobs_priority_extra_credits(tables): + tables.users_table.put_item(Item={'user_id': 'user1', 'remaining_credits': 10_003}) + + jobs = dynamo.jobs.put_jobs(user_id='user1', jobs=[{}]) assert jobs[0]['priority'] == 9999 - assert jobs[1]['priority'] == 9998 - assert jobs[2]['priority'] == 9997 - assert jobs[3]['priority'] == 9998 - assert jobs[4]['priority'] == 9997 - assert jobs[5]['priority'] == 9996 - assert jobs[6]['priority'] == 9995 - jobs.extend(dynamo.jobs.put_jobs('user2', [{}])) + jobs.extend(dynamo.jobs.put_jobs(user_id='user1', jobs=[{}])) + assert jobs[1]['priority'] == 9999 - assert jobs[7]['priority'] == int(os.environ['DEFAULT_CREDITS_PER_USER']) + jobs.extend(dynamo.jobs.put_jobs(user_id='user1', jobs=[{}] * 6)) + assert jobs[2]['priority'] == 9999 + assert jobs[3]['priority'] == 9999 + assert jobs[4]['priority'] == 9999 + assert jobs[5]['priority'] == 9998 + assert jobs[6]['priority'] == 9997 + assert jobs[7]['priority'] == 9996 +# TODO is this still the desired behavior? def test_put_jobs_priority_overflow(tables, monkeypatch): monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '10_001') many_jobs = [{} for _ in range(10_001)] From 3d2d7db2cf5bdd793994af128e65e632527f00e0 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 22 Jan 2024 14:49:05 -0900 Subject: [PATCH 045/115] priority is remaining credits --- lib/dynamo/dynamo/jobs.py | 21 ++++++++++----------- tests/test_dynamo/test_jobs.py | 26 +++++++++----------------- 2 files changed, 19 insertions(+), 28 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 9fbd89324..758628037 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -29,19 +29,22 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: remaining_credits = user_record['remaining_credits'] priority_override = user_record.get('priority_override') - previous_job_priority = None + expected_remaining_credits = remaining_credits prepared_jobs = [] for job in jobs: + credit_cost = int(get_credit_cost(job)) prepared_job = prepare_job_for_database( job=job, user_id=user_id, request_time=request_time, remaining_credits=remaining_credits, priority_override=priority_override, - previous_job_priority=previous_job_priority, + expected_remaining_credits=expected_remaining_credits, + credit_cost=credit_cost, ) prepared_jobs.append(prepared_job) - previous_job_priority = prepared_job['priority'] + if expected_remaining_credits is not None: + expected_remaining_credits -= credit_cost total_cost = sum(job['credit_cost'] for job in prepared_jobs) if remaining_credits is not None and total_cost > remaining_credits: @@ -49,6 +52,7 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: f'These jobs would cost {total_cost} credits, but you have only {remaining_credits} remaining.' ) + assert prepared_jobs[-1]['priority'] >= 0 if not dry_run: for prepared_job in prepared_jobs: table.put_item(Item=convert_floats_to_decimals(prepared_job)) @@ -65,20 +69,15 @@ def prepare_job_for_database( request_time: str, remaining_credits: Optional[float], priority_override: Optional[int], - previous_job_priority: Optional[int], + expected_remaining_credits: Optional[int], + credit_cost: int, ) -> dict: - credit_cost = get_credit_cost(job) if priority_override: priority = priority_override elif remaining_credits is None: priority = 0 - # TODO if you have > 9999 credits, you can get higher job priority by submitting jobs one at a time; - # are we OK with this behavior? - elif previous_job_priority is None: - priority = min(int(remaining_credits), 9999) else: - # TODO should we raise exception if reach 0? - priority = max(previous_job_priority - int(credit_cost), 0) + priority = min(expected_remaining_credits, 9999) return { 'job_id': str(uuid4()), 'user_id': user_id, diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 5721034ad..a842b9b7d 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -246,16 +246,18 @@ def test_put_jobs_priority_override(tables): def test_put_jobs_priority(tables): - tables.users_table.put_item(Item={'user_id': 'user1', 'remaining_credits': 30}) + tables.users_table.put_item(Item={'user_id': 'user1', 'remaining_credits': 7}) jobs = dynamo.jobs.put_jobs(user_id='user1', jobs=[{}, {}, {}]) - assert jobs[0]['priority'] == 30 - assert jobs[1]['priority'] == 29 - assert jobs[2]['priority'] == 28 + assert jobs[0]['priority'] == 7 + assert jobs[1]['priority'] == 6 + assert jobs[2]['priority'] == 5 - jobs.extend(dynamo.jobs.put_jobs(user_id='user1', jobs=[{}, {}])) - assert jobs[3]['priority'] == 27 - assert jobs[4]['priority'] == 26 + jobs.extend(dynamo.jobs.put_jobs(user_id='user1', jobs=[{}, {}, {}, {}])) + assert jobs[3]['priority'] == 4 + assert jobs[4]['priority'] == 3 + assert jobs[5]['priority'] == 2 + assert jobs[6]['priority'] == 1 def test_put_jobs_priority_extra_credits(tables): @@ -276,16 +278,6 @@ def test_put_jobs_priority_extra_credits(tables): assert jobs[7]['priority'] == 9996 -# TODO is this still the desired behavior? -def test_put_jobs_priority_overflow(tables, monkeypatch): - monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '10_001') - many_jobs = [{} for _ in range(10_001)] - jobs = dynamo.jobs.put_jobs('user3', many_jobs) - assert jobs[-1]['priority'] == 0 - assert jobs[-2]['priority'] == 0 - assert jobs[-3]['priority'] == 1 - - def test_get_job(tables): table_items = [ { From d0997cbdf8f5beeb62cc8b33f5ba702f1443f1e8 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 22 Jan 2024 15:03:42 -0900 Subject: [PATCH 046/115] handle credits as floats --- lib/dynamo/dynamo/jobs.py | 11 +++++++---- tests/test_api/test_submit_job.py | 3 +-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 758628037..4b4335b58 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -27,12 +27,15 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: user_record = dynamo.user.create_user(user_id) remaining_credits = user_record['remaining_credits'] + if remaining_credits is not None: + remaining_credits = float(remaining_credits) + priority_override = user_record.get('priority_override') expected_remaining_credits = remaining_credits prepared_jobs = [] for job in jobs: - credit_cost = int(get_credit_cost(job)) + credit_cost = get_credit_cost(job) prepared_job = prepare_job_for_database( job=job, user_id=user_id, @@ -69,15 +72,15 @@ def prepare_job_for_database( request_time: str, remaining_credits: Optional[float], priority_override: Optional[int], - expected_remaining_credits: Optional[int], - credit_cost: int, + expected_remaining_credits: Optional[float], + credit_cost: float, ) -> dict: if priority_override: priority = priority_override elif remaining_credits is None: priority = 0 else: - priority = min(expected_remaining_credits, 9999) + priority = min(int(expected_remaining_credits), 9999) return { 'job_id': str(uuid4()), 'user_id': user_id, diff --git a/tests/test_api/test_submit_job.py b/tests/test_api/test_submit_job.py index 3e3cea044..aeef351fd 100644 --- a/tests/test_api/test_submit_job.py +++ b/tests/test_api/test_submit_job.py @@ -127,8 +127,7 @@ def test_submit_exceeds_quota(client, tables, monkeypatch): response2 = submit_batch(client, batch2) assert response2.status_code == HTTPStatus.BAD_REQUEST - # TODO maybe we should fix dynamo.jobs.put_jobs so that the formatting (float vs. int) is consistent - assert response2.json['detail'] == 'These jobs would cost 10.0 credits, but you have only 5 remaining.' + assert response2.json['detail'] == 'These jobs would cost 10.0 credits, but you have only 5.0 remaining.' def test_submit_without_jobs(client): From 5bd88158f56571612a3ffbd02c7ea251117f466f Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 22 Jan 2024 15:20:20 -0900 Subject: [PATCH 047/115] refactor put_jobs to track running cost --- lib/dynamo/dynamo/jobs.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 4b4335b58..7265d70bb 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -32,24 +32,20 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: priority_override = user_record.get('priority_override') - expected_remaining_credits = remaining_credits + total_cost = 0 prepared_jobs = [] for job in jobs: - credit_cost = get_credit_cost(job) prepared_job = prepare_job_for_database( job=job, user_id=user_id, request_time=request_time, remaining_credits=remaining_credits, priority_override=priority_override, - expected_remaining_credits=expected_remaining_credits, - credit_cost=credit_cost, + running_cost=total_cost, ) prepared_jobs.append(prepared_job) - if expected_remaining_credits is not None: - expected_remaining_credits -= credit_cost + total_cost += prepared_job['credit_cost'] - total_cost = sum(job['credit_cost'] for job in prepared_jobs) if remaining_credits is not None and total_cost > remaining_credits: raise InsufficientCreditsError( f'These jobs would cost {total_cost} credits, but you have only {remaining_credits} remaining.' @@ -72,22 +68,21 @@ def prepare_job_for_database( request_time: str, remaining_credits: Optional[float], priority_override: Optional[int], - expected_remaining_credits: Optional[float], - credit_cost: float, + running_cost: float, ) -> dict: if priority_override: priority = priority_override elif remaining_credits is None: priority = 0 else: - priority = min(int(expected_remaining_credits), 9999) + priority = min(int(remaining_credits - running_cost), 9999) return { 'job_id': str(uuid4()), 'user_id': user_id, 'status_code': 'PENDING', 'execution_started': False, 'request_time': request_time, - 'credit_cost': credit_cost, + 'credit_cost': get_credit_cost(job), 'priority': priority, **job, } From e8b3cebc56d615e2188ae5d0daec9c30ede21ad0 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 22 Jan 2024 15:22:32 -0900 Subject: [PATCH 048/115] total_cost as float --- lib/dynamo/dynamo/jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 7265d70bb..76c438f50 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -32,7 +32,7 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: priority_override = user_record.get('priority_override') - total_cost = 0 + total_cost = 0.0 prepared_jobs = [] for job in jobs: prepared_job = prepare_job_for_database( From f402a3d9845d0883c7c2addfb02a25ba0d5d2cc3 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 22 Jan 2024 15:25:41 -0900 Subject: [PATCH 049/115] delete TODO --- lib/dynamo/dynamo/jobs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 76c438f50..0e028eafe 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -56,7 +56,6 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: for prepared_job in prepared_jobs: table.put_item(Item=convert_floats_to_decimals(prepared_job)) if remaining_credits is not None: - # TODO: handle ValueError from decrement_credits? or just let it propagate? dynamo.user.decrement_credits(user_id, total_cost) return prepared_jobs From 5350364fef2219dda7b7d46e0a862e1c756bdbb7 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 23 Jan 2024 17:43:07 -0900 Subject: [PATCH 050/115] draft of monthly credits reset function --- lib/dynamo/dynamo/user.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index afd248076..40258064f 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -1,4 +1,5 @@ import os +from datetime import datetime, timezone from decimal import Decimal from os import environ from typing import Optional @@ -43,3 +44,32 @@ def decrement_credits(user_id: str, cost: float) -> None: ' or remaining_credits is of the wrong data type.' ) raise + + +# TODO tests +# TODO call this function wherever we interact with user records +# TODO update the last_updated field in create_user or just call this function +# TODO make last_updated be a full timestamp (as seconds since epoch or datetime str)? +# TODO don't update if remaining_credits is null +# TODO race conditions? +def reset_credits(user_id: str) -> None: + if os.environ['MONTHLY_CREDITS_RESET'] == 'yes': + table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) + default_credits = os.environ['DEFAULT_CREDITS_PER_USER'] + current_month = datetime.now(tz=timezone.utc).strftime('%Y-%m') + user = table.get_item(Key={'user_id': user_id})['Item'] + if user['last_updated'] < current_month: + try: + table.update_item( + Key={'user_id': user_id}, + UpdateExpression='SET remaining_credits = :default_credits', + ConditionExpression='last_updated < :current_month', + ExpressionAttributeValues={ + ':default_credits': Decimal(default_credits), + ':current_month': current_month, + }, + ) + except botocore.exceptions.ClientError as e: + if e.response['Error']['Code'] == 'ConditionalCheckFailedException': + raise ValueError('Unable to update user record') # TODO better error message + raise From 96a8ba1e9c5def9db35086931efd034dcf7de4cb Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Wed, 24 Jan 2024 11:26:22 -0900 Subject: [PATCH 051/115] revise dynamo.user functions --- lib/dynamo/dynamo/user.py | 118 ++++++++++++++++++++++---------------- 1 file changed, 68 insertions(+), 50 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 40258064f..11eea1019 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -2,35 +2,86 @@ from datetime import datetime, timezone from decimal import Decimal from os import environ -from typing import Optional import botocore.exceptions from dynamo.util import DYNAMODB_RESOURCE -def get_user(user_id: str) -> Optional[dict]: - table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) - response = table.get_item(Key={'user_id': user_id}) - return response.get('Item') +class DatabaseConditionException(Exception): + """Raised when a DynamoDB condition expression check fails.""" -def create_user(user_id: str) -> dict: - table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) - if get_user(user_id): - raise ValueError(f'user {user_id} already exists') - user = {'user_id': user_id, 'remaining_credits': Decimal(os.environ['DEFAULT_CREDITS_PER_USER'])} - table.put_item(Item=user) +# TODO tests +def get_or_create_user(user_id: str) -> dict: + current_month = datetime.now(tz=timezone.utc).strftime('%Y-%m') + default_credits = Decimal(os.environ['DEFAULT_CREDITS_PER_USER']) + + users_table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) + user = users_table.get_item(Key={'user_id': user_id}).get('Item') + + if user is not None: + _reset_credits_if_needed( + user=user, + default_credits=default_credits, + current_month=current_month, + users_table=users_table, + ) + else: + user = _create_user( + user_id=user_id, + default_credits=default_credits, + current_month=current_month, + users_table=users_table, + ) + return user + + +# TODO tests +def _create_user(user_id: str, default_credits: Decimal, current_month: str, users_table) -> dict: + user = {'user_id': user_id, 'remaining_credits': default_credits, 'month_last_reset': current_month} + try: + users_table.put_item(Item=user, ConditionExpression='attribute_not_exists(user_id)') + except botocore.exceptions.ClientError as e: + if e.response['Error']['Code'] == 'ConditionalCheckFailedException': + raise DatabaseConditionException(f'Failed to create user {user_id}') + raise return user -# TODO unit tests +# TODO tests +def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> None: + if os.environ['MONTHLY_CREDITS_RESET'] == 'yes'\ + and user['month_last_reset'] < current_month\ + and user['remaining_credits'] is not None: + user_id = user['user_id'] + try: + users_table.update_item( + Key={'user_id': user_id}, + UpdateExpression='SET month_last_reset = :current_month,' + ' remaining_credits = :default_credits', + ConditionExpression='month_last_reset < :current_month' + ' AND attribute_type(remaining_credits, :number)', + ExpressionAttributeValues={ + ':default_credits': default_credits, + ':current_month': current_month, + ':number': 'N', + }, + ) + except botocore.exceptions.ClientError as e: + if e.response['Error']['Code'] == 'ConditionalCheckFailedException': + raise DatabaseConditionException(f'Failed to perform monthly credits reset for user {user_id}') + raise + + +# TODO tests +# TODO fine to not call _reset_credits_if_needed? def decrement_credits(user_id: str, cost: float) -> None: - if cost < 0: - raise ValueError(f'Cost {cost} < 0') - table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) + if cost <= 0: + raise ValueError(f'Cost {cost} <= 0') + users_table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) try: - table.update_item( + users_table.update_item( Key={'user_id': user_id}, UpdateExpression='ADD remaining_credits :delta', ConditionExpression='remaining_credits >= :cost', @@ -38,38 +89,5 @@ def decrement_credits(user_id: str, cost: float) -> None: ) except botocore.exceptions.ClientError as e: if e.response['Error']['Code'] == 'ConditionalCheckFailedException': - raise ValueError( - f"Unable to update remaining_credits for user_id '{user_id}'." - f' It is likely that the user record does not exist, remaining_credits < {cost},' - ' or remaining_credits is of the wrong data type.' - ) + raise DatabaseConditionException(f'Failed to decrement credits for user {user_id}') raise - - -# TODO tests -# TODO call this function wherever we interact with user records -# TODO update the last_updated field in create_user or just call this function -# TODO make last_updated be a full timestamp (as seconds since epoch or datetime str)? -# TODO don't update if remaining_credits is null -# TODO race conditions? -def reset_credits(user_id: str) -> None: - if os.environ['MONTHLY_CREDITS_RESET'] == 'yes': - table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) - default_credits = os.environ['DEFAULT_CREDITS_PER_USER'] - current_month = datetime.now(tz=timezone.utc).strftime('%Y-%m') - user = table.get_item(Key={'user_id': user_id})['Item'] - if user['last_updated'] < current_month: - try: - table.update_item( - Key={'user_id': user_id}, - UpdateExpression='SET remaining_credits = :default_credits', - ConditionExpression='last_updated < :current_month', - ExpressionAttributeValues={ - ':default_credits': Decimal(default_credits), - ':current_month': current_month, - }, - ) - except botocore.exceptions.ClientError as e: - if e.response['Error']['Code'] == 'ConditionalCheckFailedException': - raise ValueError('Unable to update user record') # TODO better error message - raise From ef5033747b2279d64559f095478284b590ddd84d Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Wed, 24 Jan 2024 13:15:50 -0900 Subject: [PATCH 052/115] update usages of dynamo.user --- apps/api/src/hyp3_api/handlers.py | 6 +----- lib/dynamo/dynamo/jobs.py | 4 +--- lib/dynamo/dynamo/user.py | 1 - 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index d98bc29f9..fd873a67e 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -79,11 +79,7 @@ def get_names_for_user(user): def get_user(user): - # TODO: maybe create a helper function for this since we do the same thing in `put_jobs` - user_record = dynamo.user.get_user(user) - if not user_record: - user_record = dynamo.user.create_user(user) - + user_record = dynamo.user.get_or_create_user(user) return { 'user_id': user, 'remaining_credits': user_record['remaining_credits'], diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index bb21769f9..0484edde7 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -22,9 +22,7 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: table = DYNAMODB_RESOURCE.Table(environ['JOBS_TABLE_NAME']) request_time = format_time(datetime.now(timezone.utc)) - user_record = dynamo.user.get_user(user_id) - if not user_record: - user_record = dynamo.user.create_user(user_id) + user_record = dynamo.user.get_or_create_user(user_id) remaining_credits = user_record['remaining_credits'] if remaining_credits is not None: diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 11eea1019..d3c58a40f 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -75,7 +75,6 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month # TODO tests -# TODO fine to not call _reset_credits_if_needed? def decrement_credits(user_id: str, cost: float) -> None: if cost <= 0: raise ValueError(f'Cost {cost} <= 0') From ef03dd48a2538b4c857ab1f242fbd0913cb24264 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 09:24:15 -0900 Subject: [PATCH 053/115] delete old dynamo.user tests --- tests/test_dynamo/test_user.py | 44 ++-------------------------------- 1 file changed, 2 insertions(+), 42 deletions(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 3db19dd48..bd246f80b 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -1,49 +1,9 @@ import pytest -import dynamo - - -def test_get_user(tables): - table_items = [ - { - 'user_id': 'user1', - 'remaining_credits': 5 - }, - { - 'user_id': 'user2', - 'remaining_credits': 15 - }, - ] - for item in table_items: - tables.users_table.put_item(Item=item) - - assert dynamo.user.get_user('user1') == table_items[0] - assert dynamo.user.get_user('user2') == table_items[1] - assert dynamo.user.get_user('foo') is None - - -def test_create_user(tables, monkeypatch): - monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') - - user1 = dynamo.user.create_user('user1') - assert user1 == { - 'user_id': 'user1', - 'remaining_credits': 25, - } - - user2 = dynamo.user.create_user('user2') - assert user2 == { - 'user_id': 'user2', - 'remaining_credits': 25, - } - - response = tables.users_table.scan() - assert response['Items'] == [user1, user2] - - with pytest.raises(ValueError): - dynamo.user.create_user('user1') +import dynamo.user +# TODO update? def test_decrement_credits(tables): with pytest.raises(ValueError): dynamo.user.decrement_credits('foo', 1) From 524100d1c84b6d5ea31506762e55e2cc1e02ac77 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 10:41:29 -0900 Subject: [PATCH 054/115] rename env var --- lib/dynamo/dynamo/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index d3c58a40f..7d04f3278 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -51,7 +51,7 @@ def _create_user(user_id: str, default_credits: Decimal, current_month: str, use # TODO tests def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> None: - if os.environ['MONTHLY_CREDITS_RESET'] == 'yes'\ + if os.environ['RESET_CREDITS_MONTHLY'] == 'yes'\ and user['month_last_reset'] < current_month\ and user['remaining_credits'] is not None: user_id = user['user_id'] From 6dbd5d228eb884661536d8e9f38296fab64913f7 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 10:45:19 -0900 Subject: [PATCH 055/115] rename field --- lib/dynamo/dynamo/user.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 7d04f3278..bc0dca7f8 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -39,7 +39,7 @@ def get_or_create_user(user_id: str) -> dict: # TODO tests def _create_user(user_id: str, default_credits: Decimal, current_month: str, users_table) -> dict: - user = {'user_id': user_id, 'remaining_credits': default_credits, 'month_last_reset': current_month} + user = {'user_id': user_id, 'remaining_credits': default_credits, 'month_of_last_credits_reset': current_month} try: users_table.put_item(Item=user, ConditionExpression='attribute_not_exists(user_id)') except botocore.exceptions.ClientError as e: @@ -52,15 +52,15 @@ def _create_user(user_id: str, default_credits: Decimal, current_month: str, use # TODO tests def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> None: if os.environ['RESET_CREDITS_MONTHLY'] == 'yes'\ - and user['month_last_reset'] < current_month\ + and user['month_of_last_credits_reset'] < current_month\ and user['remaining_credits'] is not None: user_id = user['user_id'] try: users_table.update_item( Key={'user_id': user_id}, - UpdateExpression='SET month_last_reset = :current_month,' + UpdateExpression='SET month_of_last_credits_reset = :current_month,' ' remaining_credits = :default_credits', - ConditionExpression='month_last_reset < :current_month' + ConditionExpression='month_of_last_credits_reset < :current_month' ' AND attribute_type(remaining_credits, :number)', ExpressionAttributeValues={ ':default_credits': default_credits, From db1683c2d83cdcf7f5ef853f65ff2a7e7cf6ec6b Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 11:26:04 -0900 Subject: [PATCH 056/115] fix bug in dynamo.user, start updating tests --- lib/dynamo/dynamo/user.py | 31 ++++++++++++++++++------------- tests/test_dynamo/test_user.py | 25 ++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index bc0dca7f8..2f73a8278 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -14,14 +14,14 @@ class DatabaseConditionException(Exception): # TODO tests def get_or_create_user(user_id: str) -> dict: - current_month = datetime.now(tz=timezone.utc).strftime('%Y-%m') + current_month = _get_current_month() default_credits = Decimal(os.environ['DEFAULT_CREDITS_PER_USER']) users_table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) user = users_table.get_item(Key={'user_id': user_id}).get('Item') if user is not None: - _reset_credits_if_needed( + user = _reset_credits_if_needed( user=user, default_credits=default_credits, current_month=current_month, @@ -37,6 +37,10 @@ def get_or_create_user(user_id: str) -> dict: return user +def _get_current_month() -> str: + return datetime.now(tz=timezone.utc).strftime('%Y-%m') + + # TODO tests def _create_user(user_id: str, default_credits: Decimal, current_month: str, users_table) -> dict: user = {'user_id': user_id, 'remaining_credits': default_credits, 'month_of_last_credits_reset': current_month} @@ -50,28 +54,29 @@ def _create_user(user_id: str, default_credits: Decimal, current_month: str, use # TODO tests -def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> None: - if os.environ['RESET_CREDITS_MONTHLY'] == 'yes'\ - and user['month_of_last_credits_reset'] < current_month\ - and user['remaining_credits'] is not None: - user_id = user['user_id'] +def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> dict: + if ( + os.environ['RESET_CREDITS_MONTHLY'] == 'yes' + and user['month_of_last_credits_reset'] < current_month + and user['remaining_credits'] is not None + ): + user['month_of_last_credits_reset'] = current_month + user['remaining_credits'] = default_credits try: - users_table.update_item( - Key={'user_id': user_id}, - UpdateExpression='SET month_of_last_credits_reset = :current_month,' - ' remaining_credits = :default_credits', + users_table.put_item( + Item=user, ConditionExpression='month_of_last_credits_reset < :current_month' ' AND attribute_type(remaining_credits, :number)', ExpressionAttributeValues={ - ':default_credits': default_credits, ':current_month': current_month, ':number': 'N', }, ) except botocore.exceptions.ClientError as e: if e.response['Error']['Code'] == 'ConditionalCheckFailedException': - raise DatabaseConditionException(f'Failed to perform monthly credits reset for user {user_id}') + raise DatabaseConditionException(f'Failed to perform monthly credits reset for user {user["user_id"]}') raise + return user # TODO tests diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index bd246f80b..9c5cdb2bc 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -1,9 +1,32 @@ +import unittest.mock +from decimal import Decimal + import pytest import dynamo.user -# TODO update? +def test_get_or_create_user_already_exists(tables, monkeypatch): + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') + tables.users_table.put_item( + Item={'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01'} + ) + + with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month: + mock_get_current_month.return_value = '2024-02' + user = dynamo.user.get_or_create_user('foo') + + assert user == {'user_id': 'foo', 'remaining_credits': Decimal(25), 'month_of_last_credits_reset': '2024-02'} + assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] + + +def test_get_or_create_user_does_not_exist(): + # TODO + assert False + + +# TODO update def test_decrement_credits(tables): with pytest.raises(ValueError): dynamo.user.decrement_credits('foo', 1) From f85d41587f8d38d5a82098038ab262a43be422d5 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 11:31:41 -0900 Subject: [PATCH 057/115] test user exists and no reset --- tests/test_dynamo/test_user.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 9c5cdb2bc..814c94867 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -6,7 +6,7 @@ import dynamo.user -def test_get_or_create_user_already_exists(tables, monkeypatch): +def test_get_or_create_user_reset(tables, monkeypatch): monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') tables.users_table.put_item( @@ -21,6 +21,21 @@ def test_get_or_create_user_already_exists(tables, monkeypatch): assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] +def test_get_or_create_user_no_reset(tables, monkeypatch): + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'no') + tables.users_table.put_item( + Item={'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01'} + ) + + with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month: + mock_get_current_month.return_value = '2024-02' + user = dynamo.user.get_or_create_user('foo') + + assert user == {'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01'} + assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] + + def test_get_or_create_user_does_not_exist(): # TODO assert False From 59a2414926b97f4213da2273d4b3672e1a45f8fb Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 11:38:46 -0900 Subject: [PATCH 058/115] test create user for get_or_create_user --- tests/test_dynamo/test_user.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 814c94867..8cddf5542 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -36,9 +36,15 @@ def test_get_or_create_user_no_reset(tables, monkeypatch): assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] -def test_get_or_create_user_does_not_exist(): - # TODO - assert False +def test_get_or_create_user_create(tables, monkeypatch): + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') + + with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month: + mock_get_current_month.return_value = '2024-02' + user = dynamo.user.get_or_create_user('foo') + + assert user == {'user_id': 'foo', 'remaining_credits': Decimal(25), 'month_of_last_credits_reset': '2024-02'} + assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] # TODO update From 9a3b60bac3ab7da6767b4e9210cc946d58db3e05 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 11:39:40 -0900 Subject: [PATCH 059/115] remove TODO --- lib/dynamo/dynamo/user.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 2f73a8278..6118ba8c3 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -12,7 +12,6 @@ class DatabaseConditionException(Exception): """Raised when a DynamoDB condition expression check fails.""" -# TODO tests def get_or_create_user(user_id: str) -> dict: current_month = _get_current_month() default_credits = Decimal(os.environ['DEFAULT_CREDITS_PER_USER']) From 29d41ec36daaa76242ad08ff714842bf77bb5f03 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 11:45:57 -0900 Subject: [PATCH 060/115] test create user --- tests/test_dynamo/test_user.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 8cddf5542..8a1ef5b50 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -47,6 +47,18 @@ def test_get_or_create_user_create(tables, monkeypatch): assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] +def test_create_user(tables): + user = dynamo.user._create_user( + user_id='foo', + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table + ) + + assert user == {'user_id': 'foo', 'remaining_credits': Decimal(25), 'month_of_last_credits_reset': '2024-02'} + assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] + + # TODO update def test_decrement_credits(tables): with pytest.raises(ValueError): From bc0d19584c8b4d2e509d95c188a19f4bca99804e Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 11:48:48 -0900 Subject: [PATCH 061/115] test create user failed --- tests/test_dynamo/test_user.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 8a1ef5b50..aafb27f24 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -59,6 +59,18 @@ def test_create_user(tables): assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] +def test_create_user_failed(tables): + tables.users_table.put_item(Item={'user_id': 'foo'}) + + with pytest.raises(dynamo.user.DatabaseConditionException): + dynamo.user._create_user( + user_id='foo', + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table + ) + + # TODO update def test_decrement_credits(tables): with pytest.raises(ValueError): From b7f7a3a226eb3acd49c8ee1d79217448524ef20a Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 11:49:43 -0900 Subject: [PATCH 062/115] remove TODO --- lib/dynamo/dynamo/user.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 6118ba8c3..fb1238617 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -40,7 +40,6 @@ def _get_current_month() -> str: return datetime.now(tz=timezone.utc).strftime('%Y-%m') -# TODO tests def _create_user(user_id: str, default_credits: Decimal, current_month: str, users_table) -> dict: user = {'user_id': user_id, 'remaining_credits': default_credits, 'month_of_last_credits_reset': current_month} try: From 0d1454ec3891e6edabee8d93756295f2e6fad333 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 11:57:23 -0900 Subject: [PATCH 063/115] add a TODO --- tests/test_dynamo/test_user.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index aafb27f24..12113ab38 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -6,6 +6,9 @@ import dynamo.user +# TODO mock the private functions when testing get_or_create_user + + def test_get_or_create_user_reset(tables, monkeypatch): monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') From 3a21dfd81de28a148de18bc2c89c475ee71efe04 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 13:47:02 -0900 Subject: [PATCH 064/115] add a TODO --- lib/dynamo/dynamo/user.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index fb1238617..e84e0d585 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -12,6 +12,7 @@ class DatabaseConditionException(Exception): """Raised when a DynamoDB condition expression check fails.""" +# TODO tests def get_or_create_user(user_id: str) -> dict: current_month = _get_current_month() default_credits = Decimal(os.environ['DEFAULT_CREDITS_PER_USER']) From 1165cc73104b247a0093b983d8e4d614b1bafce7 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 13:50:33 -0900 Subject: [PATCH 065/115] start rewriting tests for get_or_create_user, mocking private functions --- tests/test_dynamo/test_user.py | 46 +++++++++------------------------- 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 12113ab38..131df3e47 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -6,48 +6,26 @@ import dynamo.user -# TODO mock the private functions when testing get_or_create_user - - def test_get_or_create_user_reset(tables, monkeypatch): monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') - tables.users_table.put_item( - Item={'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01'} - ) + tables.users_table.put_item(Item={'user_id': 'foo'}) - with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month: + with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month, \ + unittest.mock.patch('dynamo.user._reset_credits_if_needed') as mock_reset_credits_if_needed: mock_get_current_month.return_value = '2024-02' - user = dynamo.user.get_or_create_user('foo') + mock_reset_credits_if_needed.return_value = 'reset_credits_return_value' - assert user == {'user_id': 'foo', 'remaining_credits': Decimal(25), 'month_of_last_credits_reset': '2024-02'} - assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] - - -def test_get_or_create_user_no_reset(tables, monkeypatch): - monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'no') - tables.users_table.put_item( - Item={'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01'} - ) - - with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month: - mock_get_current_month.return_value = '2024-02' user = dynamo.user.get_or_create_user('foo') - assert user == {'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01'} - assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] - - -def test_get_or_create_user_create(tables, monkeypatch): - monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') - - with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month: - mock_get_current_month.return_value = '2024-02' - user = dynamo.user.get_or_create_user('foo') + mock_get_current_month.assert_called_once_with() + mock_reset_credits_if_needed.assert_called_once_with( + user={'user_id': 'foo'}, + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table, + ) - assert user == {'user_id': 'foo', 'remaining_credits': Decimal(25), 'month_of_last_credits_reset': '2024-02'} - assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] + assert user == 'reset_credits_return_value' def test_create_user(tables): From b0bd383d2f2080c085aed3dd7f2bc4d2f29b88d6 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 13:57:39 -0900 Subject: [PATCH 066/115] finish rewriting tests for get_or_create_user --- lib/dynamo/dynamo/user.py | 1 - tests/test_dynamo/test_user.py | 21 +++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index e84e0d585..fb1238617 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -12,7 +12,6 @@ class DatabaseConditionException(Exception): """Raised when a DynamoDB condition expression check fails.""" -# TODO tests def get_or_create_user(user_id: str) -> dict: current_month = _get_current_month() default_credits = Decimal(os.environ['DEFAULT_CREDITS_PER_USER']) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 131df3e47..c04956fd9 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -28,6 +28,27 @@ def test_get_or_create_user_reset(tables, monkeypatch): assert user == 'reset_credits_return_value' +def test_get_or_create_user_create(tables, monkeypatch): + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') + + with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month, \ + unittest.mock.patch('dynamo.user._create_user') as mock_create_user: + mock_get_current_month.return_value = '2024-02' + mock_create_user.return_value = 'create_user_return_value' + + user = dynamo.user.get_or_create_user('foo') + + mock_get_current_month.assert_called_once_with() + mock_create_user.assert_called_once_with( + user_id='foo', + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table, + ) + + assert user == 'create_user_return_value' + + def test_create_user(tables): user = dynamo.user._create_user( user_id='foo', From e70219396e9e2f296d2c6a78a72cedea9aa96a11 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 14:12:39 -0900 Subject: [PATCH 067/115] test reset credits --- tests/test_dynamo/test_user.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index c04956fd9..30f4fe268 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -73,6 +73,25 @@ def test_create_user_failed(tables): ) +def test_reset_credits(tables, monkeypatch): + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') + + original_user_record = { + 'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01' + } + tables.users_table.put_item(Item=original_user_record) + + user = dynamo.user._reset_credits_if_needed( + user=original_user_record, + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table, + ) + + assert user == {'user_id': 'foo', 'remaining_credits': Decimal(25), 'month_of_last_credits_reset': '2024-02'} + assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] + + # TODO update def test_decrement_credits(tables): with pytest.raises(ValueError): From 4614ee5ae625d3a32c13154758ed53f83aef288e Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 14:21:02 -0900 Subject: [PATCH 068/115] test reset credits with env var false --- tests/test_dynamo/test_user.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 30f4fe268..1bf9ce7a0 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -92,6 +92,25 @@ def test_reset_credits(tables, monkeypatch): assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] +def test_reset_credits_no_reset(tables, monkeypatch): + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'no') + + original_user_record = { + 'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01' + } + tables.users_table.put_item(Item=original_user_record) + + user = dynamo.user._reset_credits_if_needed( + user=original_user_record, + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table, + ) + + assert user == {'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01'} + assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] + + # TODO update def test_decrement_credits(tables): with pytest.raises(ValueError): From 64dcd060f0c846496fc5b1dcad9732a9f357cde9 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 14:24:24 -0900 Subject: [PATCH 069/115] test reset credits same month --- tests/test_dynamo/test_user.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 1bf9ce7a0..86d91804e 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -111,6 +111,25 @@ def test_reset_credits_no_reset(tables, monkeypatch): assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] +def test_reset_credits_same_month(tables, monkeypatch): + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') + + original_user_record = { + 'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-02' + } + tables.users_table.put_item(Item=original_user_record) + + user = dynamo.user._reset_credits_if_needed( + user=original_user_record, + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table, + ) + + assert user == {'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-02'} + assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] + + # TODO update def test_decrement_credits(tables): with pytest.raises(ValueError): From 87e239c46b3b19766fd77aca89c7cca647c86070 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 14:28:14 -0900 Subject: [PATCH 070/115] test reset with infinite credits --- tests/test_dynamo/test_user.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 86d91804e..b0e44c5e6 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -130,6 +130,25 @@ def test_reset_credits_same_month(tables, monkeypatch): assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] +def test_reset_credits_infinite_credits(tables, monkeypatch): + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') + + original_user_record = { + 'user_id': 'foo', 'remaining_credits': None, 'month_of_last_credits_reset': '2024-01' + } + tables.users_table.put_item(Item=original_user_record) + + user = dynamo.user._reset_credits_if_needed( + user=original_user_record, + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table, + ) + + assert user == {'user_id': 'foo', 'remaining_credits': None, 'month_of_last_credits_reset': '2024-01'} + assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] + + # TODO update def test_decrement_credits(tables): with pytest.raises(ValueError): From 2ac171282c073b0b14384f48b58dd7300817d721 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 14:40:55 -0900 Subject: [PATCH 071/115] test reset credits failed check for month --- tests/test_dynamo/test_user.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index b0e44c5e6..85e0ec793 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -149,6 +149,21 @@ def test_reset_credits_infinite_credits(tables, monkeypatch): assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] +def test_reset_credits_failed_month(tables, monkeypatch): + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') + tables.users_table.put_item( + Item={'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-02'} + ) + + with pytest.raises(dynamo.user.DatabaseConditionException): + dynamo.user._reset_credits_if_needed( + user={'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01'}, + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table, + ) + + # TODO update def test_decrement_credits(tables): with pytest.raises(ValueError): From ce65e985fdc100b18c6d3fdfebfddb8f9e9b3668 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 14:47:00 -0900 Subject: [PATCH 072/115] test reset credits failed with infinite credits --- tests/test_dynamo/test_user.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 85e0ec793..d2bd00899 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -164,6 +164,21 @@ def test_reset_credits_failed_month(tables, monkeypatch): ) +def test_reset_credits_failed_infinite_credits(tables, monkeypatch): + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') + tables.users_table.put_item( + Item={'user_id': 'foo', 'remaining_credits': None, 'month_of_last_credits_reset': '2024-01'} + ) + + with pytest.raises(dynamo.user.DatabaseConditionException): + dynamo.user._reset_credits_if_needed( + user={'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01'}, + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table, + ) + + # TODO update def test_decrement_credits(tables): with pytest.raises(ValueError): From 853d276d5274102617a5be777711f3ccf0f0ddf9 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 14:49:11 -0900 Subject: [PATCH 073/115] remove TODO --- lib/dynamo/dynamo/user.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index fb1238617..a8f0cbd2c 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -51,7 +51,6 @@ def _create_user(user_id: str, default_credits: Decimal, current_month: str, use return user -# TODO tests def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> dict: if ( os.environ['RESET_CREDITS_MONTHLY'] == 'yes' From 337f5aae8c4b9fac97248199081a0175e76d489a Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 15:44:12 -0900 Subject: [PATCH 074/115] remove old test decrement credits --- tests/test_dynamo/test_user.py | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index d2bd00899..2d0d61a2e 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -178,32 +178,3 @@ def test_reset_credits_failed_infinite_credits(tables, monkeypatch): users_table=tables.users_table, ) - -# TODO update -def test_decrement_credits(tables): - with pytest.raises(ValueError): - dynamo.user.decrement_credits('foo', 1) - - user = {'user_id': 'foo', 'remaining_credits': 25} - tables.users_table.put_item(Item=user) - - with pytest.raises(ValueError): - dynamo.user.decrement_credits('foo', -1) - - dynamo.user.decrement_credits('foo', 1) - response = tables.users_table.scan() - assert response['Items'] == [{'user_id': 'foo', 'remaining_credits': 24}] - - dynamo.user.decrement_credits('foo', 4) - response = tables.users_table.scan() - assert response['Items'] == [{'user_id': 'foo', 'remaining_credits': 20}] - - # TODO should this case set remaining_credits to zero? - with pytest.raises(ValueError): - dynamo.user.decrement_credits('foo', 21) - response = tables.users_table.scan() - assert response['Items'] == [{'user_id': 'foo', 'remaining_credits': 20}] - - dynamo.user.decrement_credits('foo', 20) - response = tables.users_table.scan() - assert response['Items'] == [{'user_id': 'foo', 'remaining_credits': 0}] From d026795d0fc0f6775595ae544aacdc945a23de9c Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 15:55:17 -0900 Subject: [PATCH 075/115] add updated test decrement credits --- tests/test_dynamo/test_user.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 2d0d61a2e..6b8d5fa65 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -178,3 +178,15 @@ def test_reset_credits_failed_infinite_credits(tables, monkeypatch): users_table=tables.users_table, ) + +def test_decrement_credits(tables, monkeypatch): + tables.users_table.put_item(Item={'user_id': 'foo', 'remaining_credits': Decimal(25)}) + + dynamo.user.decrement_credits('foo', 1) + assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': Decimal(24)}] + + dynamo.user.decrement_credits('foo', 4) + assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': Decimal(20)}] + + dynamo.user.decrement_credits('foo', 20) + assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': Decimal(0)}] From 0c6966c3ef274d257f665edd35bb40fce069c24c Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 16:02:54 -0900 Subject: [PATCH 076/115] test decrement credits with cost <= 0 --- tests/test_dynamo/test_user.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 6b8d5fa65..502c8ac1f 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -179,7 +179,7 @@ def test_reset_credits_failed_infinite_credits(tables, monkeypatch): ) -def test_decrement_credits(tables, monkeypatch): +def test_decrement_credits(tables): tables.users_table.put_item(Item={'user_id': 'foo', 'remaining_credits': Decimal(25)}) dynamo.user.decrement_credits('foo', 1) @@ -190,3 +190,11 @@ def test_decrement_credits(tables, monkeypatch): dynamo.user.decrement_credits('foo', 20) assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': Decimal(0)}] + + +def test_decrement_credits_invalid_cost(): + with pytest.raises(ValueError, match=r'^Cost 0 <= 0$'): + dynamo.user.decrement_credits('foo', 0) + + with pytest.raises(ValueError, match=r'^Cost -1 <= 0$'): + dynamo.user.decrement_credits('foo', -1) From a6f3a36ceb09495cf967d1cf034059ba67055faa Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 16:14:16 -0900 Subject: [PATCH 077/115] test decrement credits cost too high --- tests/test_dynamo/test_user.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 502c8ac1f..4b18527af 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -198,3 +198,22 @@ def test_decrement_credits_invalid_cost(): with pytest.raises(ValueError, match=r'^Cost -1 <= 0$'): dynamo.user.decrement_credits('foo', -1) + + +# TODO check database condition for all the other `*failed*` tests +# TODO use scan instead of get_item? + +def test_decrement_credits_failed_cost_too_high(tables): + tables.users_table.put_item(Item={'user_id': 'foo', 'remaining_credits': Decimal(1)}) + + with pytest.raises(dynamo.user.DatabaseConditionException): + dynamo.user.decrement_credits('foo', 2) + + assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': Decimal(1)}] + + dynamo.user.decrement_credits('foo', 1) + + assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': Decimal(0)}] + + with pytest.raises(dynamo.user.DatabaseConditionException): + dynamo.user.decrement_credits('foo', 1) From c1dbdfe9751ff90416edd1f31aa60b3bde376f71 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 16:23:52 -0900 Subject: [PATCH 078/115] test decrement credits with infinite credits --- tests/test_dynamo/test_user.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 4b18527af..0eda6bef8 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -1,6 +1,7 @@ import unittest.mock from decimal import Decimal +import botocore.exceptions import pytest import dynamo.user @@ -217,3 +218,14 @@ def test_decrement_credits_failed_cost_too_high(tables): with pytest.raises(dynamo.user.DatabaseConditionException): dynamo.user.decrement_credits('foo', 1) + + +def test_decrement_credits_failed_infinite_credits(tables): + tables.users_table.put_item(Item={'user_id': 'foo', 'remaining_credits': None}) + + with pytest.raises( + botocore.exceptions.ClientError, + match=r'^An error occurred \(ValidationException\) when calling the UpdateItem operation:' + r' An operand in the update expression has an incorrect data type$' + ): + dynamo.user.decrement_credits('foo', 1) From fd180292973126e02212381ba596c0212e7efc92 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 16:26:55 -0900 Subject: [PATCH 079/115] add some asserts --- tests/test_dynamo/test_user.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 0eda6bef8..abcc45f6b 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -219,6 +219,8 @@ def test_decrement_credits_failed_cost_too_high(tables): with pytest.raises(dynamo.user.DatabaseConditionException): dynamo.user.decrement_credits('foo', 1) + assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': Decimal(0)}] + def test_decrement_credits_failed_infinite_credits(tables): tables.users_table.put_item(Item={'user_id': 'foo', 'remaining_credits': None}) @@ -229,3 +231,5 @@ def test_decrement_credits_failed_infinite_credits(tables): r' An operand in the update expression has an incorrect data type$' ): dynamo.user.decrement_credits('foo', 1) + + assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': None}] From fbceb730de95e5127deb5ad26cc452b9405da3d3 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 16:30:25 -0900 Subject: [PATCH 080/115] test decrement credits user does not exist --- tests/test_dynamo/test_user.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index abcc45f6b..6d859152a 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -233,3 +233,10 @@ def test_decrement_credits_failed_infinite_credits(tables): dynamo.user.decrement_credits('foo', 1) assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': None}] + + +def test_decrement_credits_failed_user_does_not_exist(tables): + with pytest.raises(dynamo.user.DatabaseConditionException): + dynamo.user.decrement_credits('foo', 1) + + assert tables.users_table.scan()['Items'] == [] From 2ce6b31a0178de02ab0d3c25e1b6ee48021b1a1e Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 16:33:52 -0900 Subject: [PATCH 081/115] update TODOs --- lib/dynamo/dynamo/user.py | 1 - tests/test_dynamo/test_user.py | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index a8f0cbd2c..6a7193ad8 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -76,7 +76,6 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month return user -# TODO tests def decrement_credits(user_id: str, cost: float) -> None: if cost <= 0: raise ValueError(f'Cost {cost} <= 0') diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 6d859152a..d79a9352c 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -201,6 +201,7 @@ def test_decrement_credits_invalid_cost(): dynamo.user.decrement_credits('foo', -1) +# TODO rename `*failed*` tests # TODO check database condition for all the other `*failed*` tests # TODO use scan instead of get_item? From 1f6dadb458cb0db934b68f4af4950ac6fba5ce36 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 17:51:01 -0900 Subject: [PATCH 082/115] add more asserts for users table state --- tests/test_dynamo/test_user.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index d79a9352c..9ded2dc0c 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -73,6 +73,8 @@ def test_create_user_failed(tables): users_table=tables.users_table ) + assert tables.users_table.scan()['Items'] == [{'user_id': 'foo'}] + def test_reset_credits(tables, monkeypatch): monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') @@ -164,6 +166,10 @@ def test_reset_credits_failed_month(tables, monkeypatch): users_table=tables.users_table, ) + assert tables.users_table.scan()['Items'] == [ + {'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-02'} + ] + def test_reset_credits_failed_infinite_credits(tables, monkeypatch): monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') @@ -179,6 +185,10 @@ def test_reset_credits_failed_infinite_credits(tables, monkeypatch): users_table=tables.users_table, ) + assert tables.users_table.scan()['Items'] == [ + {'user_id': 'foo', 'remaining_credits': None, 'month_of_last_credits_reset': '2024-01'} + ] + def test_decrement_credits(tables): tables.users_table.put_item(Item={'user_id': 'foo', 'remaining_credits': Decimal(25)}) @@ -193,13 +203,17 @@ def test_decrement_credits(tables): assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': Decimal(0)}] -def test_decrement_credits_invalid_cost(): +def test_decrement_credits_invalid_cost(tables): with pytest.raises(ValueError, match=r'^Cost 0 <= 0$'): dynamo.user.decrement_credits('foo', 0) + assert tables.users_table.scan()['Items'] == [] + with pytest.raises(ValueError, match=r'^Cost -1 <= 0$'): dynamo.user.decrement_credits('foo', -1) + assert tables.users_table.scan()['Items'] == [] + # TODO rename `*failed*` tests # TODO check database condition for all the other `*failed*` tests From 24731d8d94965e5490854869abd779508aa66116 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 17:52:01 -0900 Subject: [PATCH 083/115] remove TODO --- tests/test_dynamo/test_user.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 9ded2dc0c..a7f0ec42d 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -216,7 +216,6 @@ def test_decrement_credits_invalid_cost(tables): # TODO rename `*failed*` tests -# TODO check database condition for all the other `*failed*` tests # TODO use scan instead of get_item? def test_decrement_credits_failed_cost_too_high(tables): From 0de662dcddfefaebb6da0121d98007c8134772ba Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 17:55:17 -0900 Subject: [PATCH 084/115] assert with scan instead of get_item --- tests/test_dynamo/test_user.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index a7f0ec42d..15b46ad6e 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -59,7 +59,7 @@ def test_create_user(tables): ) assert user == {'user_id': 'foo', 'remaining_credits': Decimal(25), 'month_of_last_credits_reset': '2024-02'} - assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] + assert tables.users_table.scan()['Items'] == [user] def test_create_user_failed(tables): @@ -92,7 +92,7 @@ def test_reset_credits(tables, monkeypatch): ) assert user == {'user_id': 'foo', 'remaining_credits': Decimal(25), 'month_of_last_credits_reset': '2024-02'} - assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] + assert tables.users_table.scan()['Items'] == [user] def test_reset_credits_no_reset(tables, monkeypatch): @@ -111,7 +111,7 @@ def test_reset_credits_no_reset(tables, monkeypatch): ) assert user == {'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01'} - assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] + assert tables.users_table.scan()['Items'] == [user] def test_reset_credits_same_month(tables, monkeypatch): @@ -130,7 +130,7 @@ def test_reset_credits_same_month(tables, monkeypatch): ) assert user == {'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-02'} - assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] + assert tables.users_table.scan()['Items'] == [user] def test_reset_credits_infinite_credits(tables, monkeypatch): @@ -149,7 +149,7 @@ def test_reset_credits_infinite_credits(tables, monkeypatch): ) assert user == {'user_id': 'foo', 'remaining_credits': None, 'month_of_last_credits_reset': '2024-01'} - assert user == tables.users_table.get_item(Key={'user_id': 'foo'})['Item'] + assert tables.users_table.scan()['Items'] == [user] def test_reset_credits_failed_month(tables, monkeypatch): @@ -216,7 +216,6 @@ def test_decrement_credits_invalid_cost(tables): # TODO rename `*failed*` tests -# TODO use scan instead of get_item? def test_decrement_credits_failed_cost_too_high(tables): tables.users_table.put_item(Item={'user_id': 'foo', 'remaining_credits': Decimal(1)}) From c0ec2ce789a24812436f4d9d5155cb73eacb7d4d Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 17:58:38 -0900 Subject: [PATCH 085/115] rename some tests --- tests/test_dynamo/test_user.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 15b46ad6e..6f4f9c205 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -62,7 +62,7 @@ def test_create_user(tables): assert tables.users_table.scan()['Items'] == [user] -def test_create_user_failed(tables): +def test_create_user_already_exists(tables): tables.users_table.put_item(Item={'user_id': 'foo'}) with pytest.raises(dynamo.user.DatabaseConditionException): @@ -152,7 +152,7 @@ def test_reset_credits_infinite_credits(tables, monkeypatch): assert tables.users_table.scan()['Items'] == [user] -def test_reset_credits_failed_month(tables, monkeypatch): +def test_reset_credits_failed_same_month(tables, monkeypatch): monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') tables.users_table.put_item( Item={'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-02'} @@ -215,9 +215,7 @@ def test_decrement_credits_invalid_cost(tables): assert tables.users_table.scan()['Items'] == [] -# TODO rename `*failed*` tests - -def test_decrement_credits_failed_cost_too_high(tables): +def test_decrement_credits_cost_too_high(tables): tables.users_table.put_item(Item={'user_id': 'foo', 'remaining_credits': Decimal(1)}) with pytest.raises(dynamo.user.DatabaseConditionException): @@ -235,7 +233,7 @@ def test_decrement_credits_failed_cost_too_high(tables): assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': Decimal(0)}] -def test_decrement_credits_failed_infinite_credits(tables): +def test_decrement_credits_infinite_credits(tables): tables.users_table.put_item(Item={'user_id': 'foo', 'remaining_credits': None}) with pytest.raises( @@ -248,7 +246,7 @@ def test_decrement_credits_failed_infinite_credits(tables): assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': None}] -def test_decrement_credits_failed_user_does_not_exist(tables): +def test_decrement_credits_user_does_not_exist(tables): with pytest.raises(dynamo.user.DatabaseConditionException): dynamo.user.decrement_credits('foo', 1) From ff1bae71b0dcf27101f8480740d1449b2eb1acf4 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 18:19:47 -0900 Subject: [PATCH 086/115] Pass RESET_CREDITS_MONTHLY env var through --- .github/actions/deploy-hyp3/action.yml | 5 ++++- .github/workflows/deploy-credits-sandbox.yml | 2 ++ .github/workflows/deploy-daac.yml | 3 +++ .github/workflows/deploy-enterprise-test.yml | 2 ++ .github/workflows/deploy-enterprise.yml | 13 +++++++++++++ apps/api/api-cf.yml.j2 | 4 ++++ apps/main-cf.yml.j2 | 5 +++++ docs/deployments/ASF-deployment.md | 3 ++- docs/deployments/JPL-deployment.md | 3 ++- 9 files changed, 37 insertions(+), 3 deletions(-) diff --git a/.github/actions/deploy-hyp3/action.yml b/.github/actions/deploy-hyp3/action.yml index f5dbbc5ee..2223768e0 100644 --- a/.github/actions/deploy-hyp3/action.yml +++ b/.github/actions/deploy-hyp3/action.yml @@ -38,8 +38,10 @@ inputs: DEFAULT_CREDITS_PER_USER: description: "The default number of credits given to a new user" required: true + RESET_CREDITS_MONTHLY: + description: "Reset each user's remaining credits each month if and only if this value is 'yes'" JOB_FILES: - description: "Space seperated list of job spec YAMLs to include" + description: "Space separated list of job spec YAMLs to include" required: true DEFAULT_MAX_VCPUS: description: "Default maximum size for the AWS Batch compute environment" @@ -120,6 +122,7 @@ runs: $ORIGIN_ACCESS_IDENTITY_ID \ $DISTRIBUTION_URL \ DefaultCreditsPerUser='${{ inputs.DEFAULT_CREDITS_PER_USER }}' \ + ResetCreditsMonthly='${{ inputs.RESET_CREDITS_MONTHLY }}' \ DefaultMaxvCpus='${{ inputs.DEFAULT_MAX_VCPUS }}' \ ExpandedMaxvCpus='${{ inputs.EXPANDED_MAX_VCPUS }}' \ MonthlyBudget='${{ inputs.MONTHLY_BUDGET }}' \ diff --git a/.github/workflows/deploy-credits-sandbox.yml b/.github/workflows/deploy-credits-sandbox.yml index 018d0df08..4738906db 100644 --- a/.github/workflows/deploy-credits-sandbox.yml +++ b/.github/workflows/deploy-credits-sandbox.yml @@ -20,6 +20,7 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 0 + reset_credits_monthly: yes deploy_ref: refs/heads/credits-sandbox job_files: job_spec/AUTORIFT.yml job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml job_spec/INSAR_ISCE_BURST.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge @@ -62,6 +63,7 @@ jobs: SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} DEFAULT_CREDITS_PER_USER: ${{ matrix.default_credits_per_user }} + RESET_CREDITS_MONTHLY: ${{ matrix.reset_credits_monthly }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} EXPANDED_MAX_VCPUS: ${{ matrix.expanded_max_vcpus }} diff --git a/.github/workflows/deploy-daac.yml b/.github/workflows/deploy-daac.yml index bf15a74ee..a8ed03621 100644 --- a/.github/workflows/deploy-daac.yml +++ b/.github/workflows/deploy-daac.yml @@ -22,6 +22,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 1000 + reset_credits_monthly: yes deploy_ref: refs/heads/main job_files: job_spec/AUTORIFT.yml job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml job_spec/INSAR_ISCE_BURST.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge @@ -39,6 +40,7 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 1000 + reset_credits_monthly: yes deploy_ref: refs/heads/develop job_files: >- job_spec/AUTORIFT.yml @@ -87,6 +89,7 @@ jobs: SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} DEFAULT_CREDITS_PER_USER: ${{ matrix.default_credits_per_user }} + RESET_CREDITS_MONTHLY: ${{ matrix.reset_credits_monthly }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} EXPANDED_MAX_VCPUS: ${{ matrix.expanded_max_vcpus }} diff --git a/.github/workflows/deploy-enterprise-test.yml b/.github/workflows/deploy-enterprise-test.yml index f335aee20..325b3e952 100644 --- a/.github/workflows/deploy-enterprise-test.yml +++ b/.github/workflows/deploy-enterprise-test.yml @@ -20,6 +20,7 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 0 + reset_credits_monthly: no deploy_ref: refs/heads/develop job_files: >- job_spec/AUTORIFT_ITS_LIVE.yml @@ -70,6 +71,7 @@ jobs: SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} DEFAULT_CREDITS_PER_USER: ${{ matrix.default_credits_per_user }} + RESET_CREDITS_MONTHLY: ${{ matrix.reset_credits_monthly }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} EXPANDED_MAX_VCPUS: ${{ matrix.expanded_max_vcpus }} diff --git a/.github/workflows/deploy-enterprise.yml b/.github/workflows/deploy-enterprise.yml index 9167d37be..00f7c9e53 100644 --- a/.github/workflows/deploy-enterprise.yml +++ b/.github/workflows/deploy-enterprise.yml @@ -20,6 +20,7 @@ jobs: image_tag: latest product_lifetime_in_days: 45 default_credits_per_user: 0 + reset_credits_monthly: no job_files: >- job_spec/AUTORIFT_ITS_LIVE.yml job_spec/AUTORIFT_ITS_LIVE_TEST.yml @@ -38,6 +39,7 @@ jobs: image_tag: latest product_lifetime_in_days: 180 default_credits_per_user: 0 + reset_credits_monthly: no job_files: job_spec/INSAR_ISCE.yml job_spec/INSAR_ISCE_TEST.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge default_max_vcpus: 10000 @@ -53,6 +55,7 @@ jobs: image_tag: latest product_lifetime_in_days: 60 default_credits_per_user: 0 + reset_credits_monthly: no job_files: job_spec/INSAR_ISCE.yml job_spec/INSAR_ISCE_TEST.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge default_max_vcpus: 0 @@ -68,6 +71,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 + reset_credits_monthly: no job_files: job_spec/INSAR_ISCE.yml job_spec/INSAR_ISCE_TEST.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge default_max_vcpus: 1600 @@ -83,6 +87,7 @@ jobs: image_tag: latest product_lifetime_in_days: 365000 default_credits_per_user: 0 + reset_credits_monthly: no job_files: job_spec/INSAR_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -98,6 +103,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 + reset_credits_monthly: no job_files: job_spec/RTC_GAMMA.yml job_spec/WATER_MAP.yml job_spec/WATER_MAP_EQ.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -113,6 +119,7 @@ jobs: image_tag: latest product_lifetime_in_days: 90 default_credits_per_user: 0 + reset_credits_monthly: no job_files: job_spec/RTC_GAMMA.yml job_spec/WATER_MAP.yml job_spec/WATER_MAP_EQ.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 1600 @@ -128,6 +135,7 @@ jobs: image_tag: latest product_lifetime_in_days: 30 default_credits_per_user: 0 + reset_credits_monthly: no job_files: job_spec/INSAR_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -143,6 +151,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 + reset_credits_monthly: no job_files: job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -158,6 +167,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 + reset_credits_monthly: no job_files: job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -173,6 +183,7 @@ jobs: image_tag: latest product_lifetime_in_days: 30 default_credits_per_user: 0 + reset_credits_monthly: no job_files: job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -190,6 +201,7 @@ jobs: # S3 bucket, but maybe we want to allow for a backlog of products-to-be-transferred? product_lifetime_in_days: 14 default_credits_per_user: 0 + reset_credits_monthly: no job_files: job_spec/WATER_MAP.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -231,6 +243,7 @@ jobs: SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} DEFAULT_CREDITS_PER_USER: ${{ matrix.default_credits_per_user }} + RESET_CREDITS_MONTHLY: ${{ matrix.reset_credits_monthly }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} EXPANDED_MAX_VCPUS: ${{ matrix.expanded_max_vcpus }} diff --git a/apps/api/api-cf.yml.j2 b/apps/api/api-cf.yml.j2 index 0fdd8525c..1ab0fb81e 100644 --- a/apps/api/api-cf.yml.j2 +++ b/apps/api/api-cf.yml.j2 @@ -15,6 +15,9 @@ Parameters: DefaultCreditsPerUser: Type: Number + ResetCreditsMonthly: + Type: String + SystemAvailable: Type: String @@ -182,6 +185,7 @@ Resources: AUTH_PUBLIC_KEY: !Ref AuthPublicKey AUTH_ALGORITHM: !Ref AuthAlgorithm DEFAULT_CREDITS_PER_USER: !Ref DefaultCreditsPerUser + RESET_CREDITS_MONTHLY: !Ref ResetCreditsMonthly SYSTEM_AVAILABLE: !Ref SystemAvailable Code: src/ Handler: hyp3_api.lambda_handler.handler diff --git a/apps/main-cf.yml.j2 b/apps/main-cf.yml.j2 index d16ab4211..69f809824 100644 --- a/apps/main-cf.yml.j2 +++ b/apps/main-cf.yml.j2 @@ -36,6 +36,10 @@ Parameters: Type: Number MinValue: 0 + ResetCreditsMonthly: + Description: "Reset each user's remaining credits each month if and only if this value is 'yes'." + Type: String + SystemAvailable: Description: Set to false to shutdown system, API will run and provide errors to users, but will not accept jobs. Type: String @@ -116,6 +120,7 @@ Resources: AuthPublicKey: !Ref AuthPublicKey AuthAlgorithm: !Ref AuthAlgorithm DefaultCreditsPerUser: !Ref DefaultCreditsPerUser + ResetCreditsMonthly: !Ref ResetCreditsMonthly SystemAvailable: !Ref SystemAvailable {% if security_environment == 'EDC' %} VpcId: !Ref VpcId diff --git a/docs/deployments/ASF-deployment.md b/docs/deployments/ASF-deployment.md index 38c1a1489..d42368428 100644 --- a/docs/deployments/ASF-deployment.md +++ b/docs/deployments/ASF-deployment.md @@ -93,5 +93,6 @@ aws cloudformation deploy \ DomainName= \ CertificateArn= \ SecretArn= \ - DefaultCreditsPerUser=0 + DefaultCreditsPerUser=0 \ + ResetCreditsMonthly=no ``` diff --git a/docs/deployments/JPL-deployment.md b/docs/deployments/JPL-deployment.md index 45b9d25e2..5d01da13f 100644 --- a/docs/deployments/JPL-deployment.md +++ b/docs/deployments/JPL-deployment.md @@ -93,7 +93,8 @@ aws cloudformation deploy \ DomainName= \ CertificateArn= \ SecretArn= \ - DefaultCreditsPerUser=0 + DefaultCreditsPerUser=0 \ + ResetCreditsMonthly=no ``` ## 5. Post deployment From 9773ad837c33e4a017df1f40339d3720b11e6ba7 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 18:36:27 -0900 Subject: [PATCH 087/115] fix pre-existing tests --- tests/cfg.env | 1 + tests/test_dynamo/test_jobs.py | 14 +++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/cfg.env b/tests/cfg.env index a01fd7e08..7b05bb197 100644 --- a/tests/cfg.env +++ b/tests/cfg.env @@ -4,6 +4,7 @@ USERS_TABLE_NAME=hyp3-db-table-user AUTH_PUBLIC_KEY=123456789 AUTH_ALGORITHM=HS256 DEFAULT_CREDITS_PER_USER=25 +RESET_CREDITS_MONTHLY=no SYSTEM_AVAILABLE=true AWS_DEFAULT_REGION=us-west-2 AWS_ACCESS_KEY_ID=testing diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index a842b9b7d..acc505a49 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -1,4 +1,5 @@ import os +import unittest.mock from datetime import datetime, timezone from decimal import Decimal @@ -184,7 +185,10 @@ def test_query_jobs_by_type(tables): def test_put_jobs(tables): payload = [{'name': 'name1'}, {'name': 'name1'}, {'name': 'name2'}] - jobs = dynamo.jobs.put_jobs('user1', payload) + + with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month: + mock_get_current_month.return_value = '2024-02' + jobs = dynamo.jobs.put_jobs('user1', payload) assert len(jobs) == 3 for job in jobs: @@ -197,12 +201,12 @@ def test_put_jobs(tables): assert job['execution_started'] is False assert job['credit_cost'] == 1 - jobs_response = tables.jobs_table.scan() - assert jobs_response['Items'] == jobs + assert tables.jobs_table.scan()['Items'] == jobs expected_remaining_credits = int(os.environ['DEFAULT_CREDITS_PER_USER']) - 3 - users_response = tables.users_table.scan() - assert users_response['Items'] == [{'user_id': 'user1', 'remaining_credits': expected_remaining_credits}] + assert tables.users_table.scan()['Items'] == [ + {'user_id': 'user1', 'remaining_credits': expected_remaining_credits, 'month_of_last_credits_reset': '2024-02'} + ] def test_put_jobs_insufficient_credits(tables, monkeypatch): From a184f0ac02743dc842c604f692dce3d712cf7710 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Jan 2024 18:42:04 -0900 Subject: [PATCH 088/115] add noqa --- lib/dynamo/dynamo/user.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 6a7193ad8..d1a3cae00 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -54,8 +54,8 @@ def _create_user(user_id: str, default_credits: Decimal, current_month: str, use def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> dict: if ( os.environ['RESET_CREDITS_MONTHLY'] == 'yes' - and user['month_of_last_credits_reset'] < current_month - and user['remaining_credits'] is not None + and user['month_of_last_credits_reset'] < current_month # noqa: W503 + and user['remaining_credits'] is not None # noqa: W503 ): user['month_of_last_credits_reset'] = current_month user['remaining_credits'] = default_credits From 834e2cc727b5f9543e6dbe1c34445752feec746b Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 26 Jan 2024 10:01:59 -0900 Subject: [PATCH 089/115] Update apps/main-cf.yml.j2 Co-authored-by: Andrew Johnston --- apps/main-cf.yml.j2 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/main-cf.yml.j2 b/apps/main-cf.yml.j2 index 69f809824..2155f359b 100644 --- a/apps/main-cf.yml.j2 +++ b/apps/main-cf.yml.j2 @@ -39,6 +39,9 @@ Parameters: ResetCreditsMonthly: Description: "Reset each user's remaining credits each month if and only if this value is 'yes'." Type: String + AllowedValues: + - yes + - no SystemAvailable: Description: Set to false to shutdown system, API will run and provide errors to users, but will not accept jobs. From 5036ab3713dd10b6bdcce8a1d1ce4e2c8ceec777 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 26 Jan 2024 15:49:32 -0900 Subject: [PATCH 090/115] allow overriding credits per month for individual user --- lib/dynamo/dynamo/user.py | 2 +- tests/test_dynamo/test_user.py | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index d1a3cae00..dd66458e1 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -58,7 +58,7 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month and user['remaining_credits'] is not None # noqa: W503 ): user['month_of_last_credits_reset'] = current_month - user['remaining_credits'] = default_credits + user['remaining_credits'] = user.get('credits_per_month', default_credits) try: users_table.put_item( Item=user, diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 6f4f9c205..4dfada0a8 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -95,6 +95,33 @@ def test_reset_credits(tables, monkeypatch): assert tables.users_table.scan()['Items'] == [user] +def test_reset_credits_override(tables, monkeypatch): + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') + + original_user_record = { + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + 'credits_per_month': Decimal(50), + 'month_of_last_credits_reset': '2024-01', + } + tables.users_table.put_item(Item=original_user_record) + + user = dynamo.user._reset_credits_if_needed( + user=original_user_record, + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table, + ) + + assert user == { + 'user_id': 'foo', + 'remaining_credits': Decimal(50), + 'credits_per_month': Decimal(50), + 'month_of_last_credits_reset': '2024-02', + } + assert tables.users_table.scan()['Items'] == [user] + + def test_reset_credits_no_reset(tables, monkeypatch): monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'no') From cc32c676d0158db79af55d51784b9bb4c7c12540 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 26 Jan 2024 15:55:32 -0900 Subject: [PATCH 091/115] reset credits for all enterprise deployments --- .github/workflows/deploy-enterprise.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/deploy-enterprise.yml b/.github/workflows/deploy-enterprise.yml index 00f7c9e53..bdb9c79cd 100644 --- a/.github/workflows/deploy-enterprise.yml +++ b/.github/workflows/deploy-enterprise.yml @@ -20,7 +20,7 @@ jobs: image_tag: latest product_lifetime_in_days: 45 default_credits_per_user: 0 - reset_credits_monthly: no + reset_credits_monthly: yes job_files: >- job_spec/AUTORIFT_ITS_LIVE.yml job_spec/AUTORIFT_ITS_LIVE_TEST.yml @@ -39,7 +39,7 @@ jobs: image_tag: latest product_lifetime_in_days: 180 default_credits_per_user: 0 - reset_credits_monthly: no + reset_credits_monthly: yes job_files: job_spec/INSAR_ISCE.yml job_spec/INSAR_ISCE_TEST.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge default_max_vcpus: 10000 @@ -55,7 +55,7 @@ jobs: image_tag: latest product_lifetime_in_days: 60 default_credits_per_user: 0 - reset_credits_monthly: no + reset_credits_monthly: yes job_files: job_spec/INSAR_ISCE.yml job_spec/INSAR_ISCE_TEST.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge default_max_vcpus: 0 @@ -71,7 +71,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: no + reset_credits_monthly: yes job_files: job_spec/INSAR_ISCE.yml job_spec/INSAR_ISCE_TEST.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge default_max_vcpus: 1600 @@ -87,7 +87,7 @@ jobs: image_tag: latest product_lifetime_in_days: 365000 default_credits_per_user: 0 - reset_credits_monthly: no + reset_credits_monthly: yes job_files: job_spec/INSAR_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -103,7 +103,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: no + reset_credits_monthly: yes job_files: job_spec/RTC_GAMMA.yml job_spec/WATER_MAP.yml job_spec/WATER_MAP_EQ.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -119,7 +119,7 @@ jobs: image_tag: latest product_lifetime_in_days: 90 default_credits_per_user: 0 - reset_credits_monthly: no + reset_credits_monthly: yes job_files: job_spec/RTC_GAMMA.yml job_spec/WATER_MAP.yml job_spec/WATER_MAP_EQ.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 1600 @@ -135,7 +135,7 @@ jobs: image_tag: latest product_lifetime_in_days: 30 default_credits_per_user: 0 - reset_credits_monthly: no + reset_credits_monthly: yes job_files: job_spec/INSAR_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -151,7 +151,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: no + reset_credits_monthly: yes job_files: job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -167,7 +167,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: no + reset_credits_monthly: yes job_files: job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -183,7 +183,7 @@ jobs: image_tag: latest product_lifetime_in_days: 30 default_credits_per_user: 0 - reset_credits_monthly: no + reset_credits_monthly: yes job_files: job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -201,7 +201,7 @@ jobs: # S3 bucket, but maybe we want to allow for a backlog of products-to-be-transferred? product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: no + reset_credits_monthly: yes job_files: job_spec/WATER_MAP.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 From 17b8a037c451ac43e91d520012ba050bec189cf8 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 26 Jan 2024 16:09:28 -0900 Subject: [PATCH 092/115] mark RESET_CREDITS_MONTHLY required in deploy action --- .github/actions/deploy-hyp3/action.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/actions/deploy-hyp3/action.yml b/.github/actions/deploy-hyp3/action.yml index 2223768e0..d4cafa964 100644 --- a/.github/actions/deploy-hyp3/action.yml +++ b/.github/actions/deploy-hyp3/action.yml @@ -40,6 +40,7 @@ inputs: required: true RESET_CREDITS_MONTHLY: description: "Reset each user's remaining credits each month if and only if this value is 'yes'" + required: true JOB_FILES: description: "Space separated list of job spec YAMLs to include" required: true From 898baa078350b6dcac574cb80ed5514f3fdbabdf Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 26 Jan 2024 16:33:52 -0900 Subject: [PATCH 093/115] use true/false for reset_credits_monthly --- .github/actions/deploy-hyp3/action.yml | 2 +- .github/workflows/deploy-credits-sandbox.yml | 2 +- .github/workflows/deploy-daac.yml | 4 ++-- .github/workflows/deploy-enterprise-test.yml | 2 +- .github/workflows/deploy-enterprise.yml | 24 ++++++++++---------- apps/main-cf.yml.j2 | 6 ++--- lib/dynamo/dynamo/user.py | 2 +- tests/cfg.env | 2 +- tests/test_dynamo/test_user.py | 14 ++++++------ 9 files changed, 29 insertions(+), 29 deletions(-) diff --git a/.github/actions/deploy-hyp3/action.yml b/.github/actions/deploy-hyp3/action.yml index d4cafa964..f1c5649b4 100644 --- a/.github/actions/deploy-hyp3/action.yml +++ b/.github/actions/deploy-hyp3/action.yml @@ -39,7 +39,7 @@ inputs: description: "The default number of credits given to a new user" required: true RESET_CREDITS_MONTHLY: - description: "Reset each user's remaining credits each month if and only if this value is 'yes'" + description: "Whether to reset each user's remaining credits each month" required: true JOB_FILES: description: "Space separated list of job spec YAMLs to include" diff --git a/.github/workflows/deploy-credits-sandbox.yml b/.github/workflows/deploy-credits-sandbox.yml index 4738906db..4bed2ae9a 100644 --- a/.github/workflows/deploy-credits-sandbox.yml +++ b/.github/workflows/deploy-credits-sandbox.yml @@ -20,7 +20,7 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: yes + reset_credits_monthly: true deploy_ref: refs/heads/credits-sandbox job_files: job_spec/AUTORIFT.yml job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml job_spec/INSAR_ISCE_BURST.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge diff --git a/.github/workflows/deploy-daac.yml b/.github/workflows/deploy-daac.yml index a8ed03621..57c107737 100644 --- a/.github/workflows/deploy-daac.yml +++ b/.github/workflows/deploy-daac.yml @@ -22,7 +22,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 1000 - reset_credits_monthly: yes + reset_credits_monthly: true deploy_ref: refs/heads/main job_files: job_spec/AUTORIFT.yml job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml job_spec/INSAR_ISCE_BURST.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge @@ -40,7 +40,7 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 1000 - reset_credits_monthly: yes + reset_credits_monthly: true deploy_ref: refs/heads/develop job_files: >- job_spec/AUTORIFT.yml diff --git a/.github/workflows/deploy-enterprise-test.yml b/.github/workflows/deploy-enterprise-test.yml index 325b3e952..0bd80a138 100644 --- a/.github/workflows/deploy-enterprise-test.yml +++ b/.github/workflows/deploy-enterprise-test.yml @@ -20,7 +20,7 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: no + reset_credits_monthly: false deploy_ref: refs/heads/develop job_files: >- job_spec/AUTORIFT_ITS_LIVE.yml diff --git a/.github/workflows/deploy-enterprise.yml b/.github/workflows/deploy-enterprise.yml index bdb9c79cd..8f1ee1768 100644 --- a/.github/workflows/deploy-enterprise.yml +++ b/.github/workflows/deploy-enterprise.yml @@ -20,7 +20,7 @@ jobs: image_tag: latest product_lifetime_in_days: 45 default_credits_per_user: 0 - reset_credits_monthly: yes + reset_credits_monthly: true job_files: >- job_spec/AUTORIFT_ITS_LIVE.yml job_spec/AUTORIFT_ITS_LIVE_TEST.yml @@ -39,7 +39,7 @@ jobs: image_tag: latest product_lifetime_in_days: 180 default_credits_per_user: 0 - reset_credits_monthly: yes + reset_credits_monthly: true job_files: job_spec/INSAR_ISCE.yml job_spec/INSAR_ISCE_TEST.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge default_max_vcpus: 10000 @@ -55,7 +55,7 @@ jobs: image_tag: latest product_lifetime_in_days: 60 default_credits_per_user: 0 - reset_credits_monthly: yes + reset_credits_monthly: true job_files: job_spec/INSAR_ISCE.yml job_spec/INSAR_ISCE_TEST.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge default_max_vcpus: 0 @@ -71,7 +71,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: yes + reset_credits_monthly: true job_files: job_spec/INSAR_ISCE.yml job_spec/INSAR_ISCE_TEST.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge default_max_vcpus: 1600 @@ -87,7 +87,7 @@ jobs: image_tag: latest product_lifetime_in_days: 365000 default_credits_per_user: 0 - reset_credits_monthly: yes + reset_credits_monthly: true job_files: job_spec/INSAR_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -103,7 +103,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: yes + reset_credits_monthly: true job_files: job_spec/RTC_GAMMA.yml job_spec/WATER_MAP.yml job_spec/WATER_MAP_EQ.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -119,7 +119,7 @@ jobs: image_tag: latest product_lifetime_in_days: 90 default_credits_per_user: 0 - reset_credits_monthly: yes + reset_credits_monthly: true job_files: job_spec/RTC_GAMMA.yml job_spec/WATER_MAP.yml job_spec/WATER_MAP_EQ.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 1600 @@ -135,7 +135,7 @@ jobs: image_tag: latest product_lifetime_in_days: 30 default_credits_per_user: 0 - reset_credits_monthly: yes + reset_credits_monthly: true job_files: job_spec/INSAR_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -151,7 +151,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: yes + reset_credits_monthly: true job_files: job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -167,7 +167,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: yes + reset_credits_monthly: true job_files: job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -183,7 +183,7 @@ jobs: image_tag: latest product_lifetime_in_days: 30 default_credits_per_user: 0 - reset_credits_monthly: yes + reset_credits_monthly: true job_files: job_spec/INSAR_GAMMA.yml job_spec/RTC_GAMMA.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 @@ -201,7 +201,7 @@ jobs: # S3 bucket, but maybe we want to allow for a backlog of products-to-be-transferred? product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: yes + reset_credits_monthly: true job_files: job_spec/WATER_MAP.yml instance_types: r6id.xlarge,r6id.2xlarge,r6id.4xlarge,r6id.8xlarge,r6idn.xlarge,r6idn.2xlarge,r6idn.4xlarge,r6idn.8xlarge default_max_vcpus: 640 diff --git a/apps/main-cf.yml.j2 b/apps/main-cf.yml.j2 index 2155f359b..8ff11876e 100644 --- a/apps/main-cf.yml.j2 +++ b/apps/main-cf.yml.j2 @@ -37,11 +37,11 @@ Parameters: MinValue: 0 ResetCreditsMonthly: - Description: "Reset each user's remaining credits each month if and only if this value is 'yes'." + Description: "Whether to reset each user's remaining credits each month" Type: String AllowedValues: - - yes - - no + - false + - true SystemAvailable: Description: Set to false to shutdown system, API will run and provide errors to users, but will not accept jobs. diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index dd66458e1..ac5b3c444 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -53,7 +53,7 @@ def _create_user(user_id: str, default_credits: Decimal, current_month: str, use def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> dict: if ( - os.environ['RESET_CREDITS_MONTHLY'] == 'yes' + os.environ['RESET_CREDITS_MONTHLY'] == 'true' and user['month_of_last_credits_reset'] < current_month # noqa: W503 and user['remaining_credits'] is not None # noqa: W503 ): diff --git a/tests/cfg.env b/tests/cfg.env index 7b05bb197..42d4b557b 100644 --- a/tests/cfg.env +++ b/tests/cfg.env @@ -4,7 +4,7 @@ USERS_TABLE_NAME=hyp3-db-table-user AUTH_PUBLIC_KEY=123456789 AUTH_ALGORITHM=HS256 DEFAULT_CREDITS_PER_USER=25 -RESET_CREDITS_MONTHLY=no +RESET_CREDITS_MONTHLY=false SYSTEM_AVAILABLE=true AWS_DEFAULT_REGION=us-west-2 AWS_ACCESS_KEY_ID=testing diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 4dfada0a8..0dbab7679 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -77,7 +77,7 @@ def test_create_user_already_exists(tables): def test_reset_credits(tables, monkeypatch): - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'true') original_user_record = { 'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01' @@ -96,7 +96,7 @@ def test_reset_credits(tables, monkeypatch): def test_reset_credits_override(tables, monkeypatch): - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'true') original_user_record = { 'user_id': 'foo', @@ -123,7 +123,7 @@ def test_reset_credits_override(tables, monkeypatch): def test_reset_credits_no_reset(tables, monkeypatch): - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'no') + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'false') original_user_record = { 'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01' @@ -142,7 +142,7 @@ def test_reset_credits_no_reset(tables, monkeypatch): def test_reset_credits_same_month(tables, monkeypatch): - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'true') original_user_record = { 'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-02' @@ -161,7 +161,7 @@ def test_reset_credits_same_month(tables, monkeypatch): def test_reset_credits_infinite_credits(tables, monkeypatch): - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'true') original_user_record = { 'user_id': 'foo', 'remaining_credits': None, 'month_of_last_credits_reset': '2024-01' @@ -180,7 +180,7 @@ def test_reset_credits_infinite_credits(tables, monkeypatch): def test_reset_credits_failed_same_month(tables, monkeypatch): - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'true') tables.users_table.put_item( Item={'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-02'} ) @@ -199,7 +199,7 @@ def test_reset_credits_failed_same_month(tables, monkeypatch): def test_reset_credits_failed_infinite_credits(tables, monkeypatch): - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'yes') + monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'true') tables.users_table.put_item( Item={'user_id': 'foo', 'remaining_credits': None, 'month_of_last_credits_reset': '2024-01'} ) From ad5bbe072fda10e53003e4c1d915bd4884aaa354 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 29 Jan 2024 09:45:59 -0900 Subject: [PATCH 094/115] decrement credits before creating job records --- lib/dynamo/dynamo/jobs.py | 4 ++-- tests/test_dynamo/test_jobs.py | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 0484edde7..3d19263b6 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -51,11 +51,11 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: assert prepared_jobs[-1]['priority'] >= 0 if not dry_run: + if remaining_credits is not None: + dynamo.user.decrement_credits(user_id, total_cost) with table.batch_writer() as batch: for prepared_job in prepared_jobs: batch.put_item(Item=convert_floats_to_decimals(prepared_job)) - if remaining_credits is not None: - dynamo.user.decrement_credits(user_id, total_cost) return prepared_jobs diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index acc505a49..95c3d0f14 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -282,6 +282,17 @@ def test_put_jobs_priority_extra_credits(tables): assert jobs[7]['priority'] == 9996 +def test_put_jobs_decrement_credits_failure(tables): + def mock_decrement_credits(*args): + raise ValueError('test error') + + with unittest.mock.patch('dynamo.user.decrement_credits', mock_decrement_credits): + with pytest.raises(ValueError, match=r'^test error$'): + dynamo.jobs.put_jobs('foo', [{'name': 'job1'}]) + + assert tables.jobs_table.scan()['Items'] == [] + + def test_get_job(tables): table_items = [ { From eb8b3e83adfe14f359c70faf47e8c69ef8686785 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 29 Jan 2024 09:51:11 -0900 Subject: [PATCH 095/115] refactor mock side effect --- tests/test_dynamo/test_jobs.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 95c3d0f14..329c473cc 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -283,11 +283,9 @@ def test_put_jobs_priority_extra_credits(tables): def test_put_jobs_decrement_credits_failure(tables): - def mock_decrement_credits(*args): - raise ValueError('test error') - - with unittest.mock.patch('dynamo.user.decrement_credits', mock_decrement_credits): - with pytest.raises(ValueError, match=r'^test error$'): + with unittest.mock.patch('dynamo.user.decrement_credits') as mock_decrement_credits: + mock_decrement_credits.side_effect = Exception('test error') + with pytest.raises(Exception, match=r'^test error$'): dynamo.jobs.put_jobs('foo', [{'name': 'job1'}]) assert tables.jobs_table.scan()['Items'] == [] From c521bb1c62c45ba8c56b81428d3acd985f3917a9 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 29 Jan 2024 11:54:13 -0900 Subject: [PATCH 096/115] prefix private dynamo.jobs functions with underscore --- lib/dynamo/dynamo/jobs.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 3d19263b6..0b4a10270 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -13,7 +13,7 @@ class InsufficientCreditsError(Exception): """Raised when trying to submit jobs whose total cost exceeds the user's remaining credits.""" -def get_credit_cost(job: dict) -> float: +def _get_credit_cost(job: dict) -> float: return 1.0 @@ -33,7 +33,7 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: total_cost = 0.0 prepared_jobs = [] for job in jobs: - prepared_job = prepare_job_for_database( + prepared_job = _prepare_job_for_database( job=job, user_id=user_id, request_time=request_time, @@ -60,7 +60,7 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: return prepared_jobs -def prepare_job_for_database( +def _prepare_job_for_database( job: dict, user_id: str, request_time: str, @@ -80,7 +80,7 @@ def prepare_job_for_database( 'status_code': 'PENDING', 'execution_started': False, 'request_time': request_time, - 'credit_cost': get_credit_cost(job), + 'credit_cost': _get_credit_cost(job), 'priority': priority, **job, } From 7cbdd94b0ba40a24848b576abb776626d454a18c Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 29 Jan 2024 12:07:50 -0900 Subject: [PATCH 097/115] improve test_put_jobs_insufficient_credits --- tests/test_dynamo/test_jobs.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 329c473cc..b8c9dba51 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -183,6 +183,7 @@ def test_query_jobs_by_type(tables): assert list_have_same_elements(response, table_items[:2]) +# TODO test user already exists, etc. def test_put_jobs(tables): payload = [{'name': 'name1'}, {'name': 'name1'}, {'name': 'name2'}] @@ -213,8 +214,15 @@ def test_put_jobs_insufficient_credits(tables, monkeypatch): monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '1') payload = [{'name': 'name1'}, {'name': 'name2'}] - with pytest.raises(dynamo.jobs.InsufficientCreditsError): - dynamo.jobs.put_jobs('user1', payload) + with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month: + mock_get_current_month.return_value = '2024-02' + with pytest.raises(dynamo.jobs.InsufficientCreditsError): + dynamo.jobs.put_jobs('user1', payload) + + assert tables.jobs_table.scan()['Items'] == [] + assert tables.users_table.scan()['Items'] == [ + {'user_id': 'user1', 'remaining_credits': 1, 'month_of_last_credits_reset': '2024-02'} + ] def test_put_jobs_infinite_credits(tables, monkeypatch): From e68dca8896f4d9ff31352da8e20988b3b8e4b65a Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 29 Jan 2024 13:44:42 -0900 Subject: [PATCH 098/115] set DEFAULT_CREDITS_PER_USER in test_put_jobs --- tests/test_dynamo/test_jobs.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index b8c9dba51..b5e0dcf97 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -184,7 +184,8 @@ def test_query_jobs_by_type(tables): # TODO test user already exists, etc. -def test_put_jobs(tables): +def test_put_jobs(tables, monkeypatch): + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '10') payload = [{'name': 'name1'}, {'name': 'name1'}, {'name': 'name2'}] with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month: @@ -204,9 +205,8 @@ def test_put_jobs(tables): assert tables.jobs_table.scan()['Items'] == jobs - expected_remaining_credits = int(os.environ['DEFAULT_CREDITS_PER_USER']) - 3 assert tables.users_table.scan()['Items'] == [ - {'user_id': 'user1', 'remaining_credits': expected_remaining_credits, 'month_of_last_credits_reset': '2024-02'} + {'user_id': 'user1', 'remaining_credits': 7, 'month_of_last_credits_reset': '2024-02'} ] From 0e059013545ac82332c60e4dcfc8aa2f30433c4d Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 29 Jan 2024 13:58:48 -0900 Subject: [PATCH 099/115] test put_jobs when user exists --- tests/test_dynamo/test_jobs.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index b5e0dcf97..5b13fe346 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -183,15 +183,17 @@ def test_query_jobs_by_type(tables): assert list_have_same_elements(response, table_items[:2]) -# TODO test user already exists, etc. def test_put_jobs(tables, monkeypatch): monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '10') payload = [{'name': 'name1'}, {'name': 'name1'}, {'name': 'name2'}] with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month: mock_get_current_month.return_value = '2024-02' + jobs = dynamo.jobs.put_jobs('user1', payload) + mock_get_current_month.assert_called_once_with() + assert len(jobs) == 3 for job in jobs: assert set(job.keys()) == { @@ -210,6 +212,16 @@ def test_put_jobs(tables, monkeypatch): ] +def test_put_jobs_user_exists(tables): + tables.users_table.put_item(Item={'user_id': 'user1', 'remaining_credits': 5}) + + jobs = dynamo.jobs.put_jobs('user1', [{}, {}]) + + assert len(jobs) == 2 + assert tables.jobs_table.scan()['Items'] == jobs + assert tables.users_table.scan()['Items'] == [{'user_id': 'user1', 'remaining_credits': 3}] + + def test_put_jobs_insufficient_credits(tables, monkeypatch): monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '1') payload = [{'name': 'name1'}, {'name': 'name2'}] From dcdd5654563ed3c2039cf8801eec03de1ee72ebf Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 29 Jan 2024 14:09:34 -0900 Subject: [PATCH 100/115] change username in test --- tests/test_dynamo/test_jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 5b13fe346..41e637227 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -306,7 +306,7 @@ def test_put_jobs_decrement_credits_failure(tables): with unittest.mock.patch('dynamo.user.decrement_credits') as mock_decrement_credits: mock_decrement_credits.side_effect = Exception('test error') with pytest.raises(Exception, match=r'^test error$'): - dynamo.jobs.put_jobs('foo', [{'name': 'job1'}]) + dynamo.jobs.put_jobs('user1', [{'name': 'job1'}]) assert tables.jobs_table.scan()['Items'] == [] From 9170884d370350900a881b8a4926ec09939e4b20 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 29 Jan 2024 14:18:58 -0900 Subject: [PATCH 101/115] remove TODO --- lib/dynamo/dynamo/jobs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 0b4a10270..3214136c2 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -17,7 +17,6 @@ def _get_credit_cost(job: dict) -> float: return 1.0 -# TODO add/update tests def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: table = DYNAMODB_RESOURCE.Table(environ['JOBS_TABLE_NAME']) request_time = format_time(datetime.now(timezone.utc)) From dbc886a3e0ca8813f515653ba3296e87950cb334 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 29 Jan 2024 14:37:55 -0900 Subject: [PATCH 102/115] rename test --- tests/test_api/test_submit_job.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_api/test_submit_job.py b/tests/test_api/test_submit_job.py index aeef351fd..1a0a80f05 100644 --- a/tests/test_api/test_submit_job.py +++ b/tests/test_api/test_submit_job.py @@ -112,7 +112,7 @@ def test_submit_many_jobs(client, tables): assert response.status_code == HTTPStatus.BAD_REQUEST -def test_submit_exceeds_quota(client, tables, monkeypatch): +def test_submit_exceeds_remaining_credits(client, tables, monkeypatch): login(client) monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') From b6b39359e84ef01c8e353f4525566240133c0efe Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 29 Jan 2024 14:46:10 -0900 Subject: [PATCH 103/115] remove unused import --- tests/test_dynamo/test_jobs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 41e637227..ee9833856 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -1,4 +1,3 @@ -import os import unittest.mock from datetime import datetime, timezone from decimal import Decimal From 91b8315458f82b7e6db6fb03077ad4557bd6627c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 30 Jan 2024 19:48:45 +0000 Subject: [PATCH 104/115] Bump cryptography from 42.0.1 to 42.0.2 Bumps [cryptography](https://github.com/pyca/cryptography) from 42.0.1 to 42.0.2. - [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst) - [Commits](https://github.com/pyca/cryptography/compare/42.0.1...42.0.2) --- updated-dependencies: - dependency-name: cryptography dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- requirements-apps-api-binary.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-apps-api-binary.txt b/requirements-apps-api-binary.txt index d0384ae0d..1041fe690 100644 --- a/requirements-apps-api-binary.txt +++ b/requirements-apps-api-binary.txt @@ -1 +1 @@ -cryptography==42.0.1 +cryptography==42.0.2 From 23c8f5f2557fed2baa45800fefd0a556da699ecd Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 30 Jan 2024 10:52:14 -0900 Subject: [PATCH 105/115] changelog --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30772e450..b306b4f69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [6.0.0] +### Added +- The `job` object returned by the `/jobs` API endpoints now includes a `credit_cost` attribute, which represents the job's cost in credits. + +### Changed +- The `quota` attribute of the `user` object returned by the `/user` API endpoint has been replaced by a `remaining_credits` attribute, which represents the user's remaining credits. +- HyP3's monthly quota system has been replaced by a credits system. Previously, HyP3 provided each user with a certain number of jobs per month. Now, each job costs a particular number of credits, and users spend credits when they submit jobs. This release assigns every job a cost of 1 credit, but future releases will assign a different credit cost to each job type. Additionally, the main production deployment (`https://hyp3-api.asf.alaska.edu`) resets each user's balance to 1,000 credits each month, effectively granting each user 1,000 jobs per month. Therefore, users should not notice any difference when ordering jobs via ASF's On Demand service at . + ## [5.0.4] ### Added - `INSAR_ISCE_BURST` jobs are now available in the azdwr-hyp3 deployment. From 2c9aa86e5a86820b590d489cd51564ae5d4d89e0 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 30 Jan 2024 10:56:21 -0900 Subject: [PATCH 106/115] changelog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b306b4f69..f08f3c814 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,12 +5,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [6.0.0] + +HyP3's monthly quota system has been replaced by a credits system. Previously, HyP3 provided each user with a certain number of jobs per month. Now, each job costs a particular number of credits, and users spend credits when they submit jobs. This release assigns every job a cost of 1 credit, but future releases will assign a different credit cost to each job type. Additionally, the main production deployment (`https://hyp3-api.asf.alaska.edu`) resets each user's balance to 1,000 credits each month, effectively granting each user 1,000 jobs per month. Therefore, users should not notice any difference when ordering jobs via ASF's On Demand service at . + ### Added - The `job` object returned by the `/jobs` API endpoints now includes a `credit_cost` attribute, which represents the job's cost in credits. ### Changed - The `quota` attribute of the `user` object returned by the `/user` API endpoint has been replaced by a `remaining_credits` attribute, which represents the user's remaining credits. -- HyP3's monthly quota system has been replaced by a credits system. Previously, HyP3 provided each user with a certain number of jobs per month. Now, each job costs a particular number of credits, and users spend credits when they submit jobs. This release assigns every job a cost of 1 credit, but future releases will assign a different credit cost to each job type. Additionally, the main production deployment (`https://hyp3-api.asf.alaska.edu`) resets each user's balance to 1,000 credits each month, effectively granting each user 1,000 jobs per month. Therefore, users should not notice any difference when ordering jobs via ASF's On Demand service at . ## [5.0.4] ### Added From 4f5f479dfcbc911546bc0fe0f39cb23cedd2de23 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 31 Jan 2024 19:34:34 +0000 Subject: [PATCH 107/115] Bump boto3 from 1.34.29 to 1.34.31 Bumps [boto3](https://github.com/boto/boto3) from 1.34.29 to 1.34.31. - [Release notes](https://github.com/boto/boto3/releases) - [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst) - [Commits](https://github.com/boto/boto3/compare/1.34.29...1.34.31) --- updated-dependencies: - dependency-name: boto3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- requirements-all.txt | 2 +- requirements-apps-disable-private-dns.txt | 2 +- requirements-apps-start-execution-manager.txt | 2 +- requirements-apps-start-execution-worker.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements-all.txt b/requirements-all.txt index d64f760f0..248769433 100644 --- a/requirements-all.txt +++ b/requirements-all.txt @@ -5,7 +5,7 @@ -r requirements-apps-start-execution-worker.txt -r requirements-apps-disable-private-dns.txt -r requirements-apps-update-db.txt -boto3==1.34.29 +boto3==1.34.31 jinja2==3.1.3 moto[dynamodb]==5.0.0 pytest==8.0.0 diff --git a/requirements-apps-disable-private-dns.txt b/requirements-apps-disable-private-dns.txt index b9c720e80..67ae4b2fc 100644 --- a/requirements-apps-disable-private-dns.txt +++ b/requirements-apps-disable-private-dns.txt @@ -1 +1 @@ -boto3==1.34.29 +boto3==1.34.31 diff --git a/requirements-apps-start-execution-manager.txt b/requirements-apps-start-execution-manager.txt index 20ec2b546..99bff074b 100644 --- a/requirements-apps-start-execution-manager.txt +++ b/requirements-apps-start-execution-manager.txt @@ -1,3 +1,3 @@ -boto3==1.34.29 +boto3==1.34.31 ./lib/dynamo/ ./lib/lambda_logging/ diff --git a/requirements-apps-start-execution-worker.txt b/requirements-apps-start-execution-worker.txt index cfe814906..babdf3ad1 100644 --- a/requirements-apps-start-execution-worker.txt +++ b/requirements-apps-start-execution-worker.txt @@ -1,2 +1,2 @@ -boto3==1.34.29 +boto3==1.34.31 ./lib/lambda_logging/ From 215019ee4391dc210b34e87c43a2d450c1021338 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Wed, 31 Jan 2024 15:03:47 -0900 Subject: [PATCH 108/115] fix cloudwatch metric used to monitor for api 5xx errors --- CHANGELOG.md | 5 +++++ apps/api/api-cf.yml.j2 | 4 ++-- apps/main-cf.yml.j2 | 2 +- apps/monitoring-cf.yml.j2 | 8 ++++---- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f08f3c814..d825ac88b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [6.0.1] + +### Fixed +- The `monitoring` module now successfully sends SNS notifications when the API encounters HTTP 5xx errors. Fixes [#2044](https://github.com/ASFHyP3/hyp3/issues/2044). + ## [6.0.0] HyP3's monthly quota system has been replaced by a credits system. Previously, HyP3 provided each user with a certain number of jobs per month. Now, each job costs a particular number of credits, and users spend credits when they submit jobs. This release assigns every job a cost of 1 credit, but future releases will assign a different credit cost to each job type. Additionally, the main production deployment (`https://hyp3-api.asf.alaska.edu`) resets each user's balance to 1,000 credits each month, effectively granting each user 1,000 jobs per month. Therefore, users should not notice any difference when ordering jobs via ASF's On Demand service at . diff --git a/apps/api/api-cf.yml.j2 b/apps/api/api-cf.yml.j2 index 1ab0fb81e..44e45f4d6 100644 --- a/apps/api/api-cf.yml.j2 +++ b/apps/api/api-cf.yml.j2 @@ -44,8 +44,8 @@ Outputs: Url: Value: {{ '!Sub "https://${RestApi}.execute-api.${AWS::Region}.amazonaws.com/${Stage}/ui"' if security_environment == 'EDC' else '!Sub "https://${CustomDomainName}/ui"' }} - ApiId: - Value: !Ref RestApi + ApiName: + Value: !GetAtt RestApi.RestApiId Resources: diff --git a/apps/main-cf.yml.j2 b/apps/main-cf.yml.j2 index 8ff11876e..9dcb3b3e1 100644 --- a/apps/main-cf.yml.j2 +++ b/apps/main-cf.yml.j2 @@ -197,7 +197,7 @@ Resources: Properties: Parameters: StepFunctionArn: !GetAtt StepFunction.Outputs.StepFunctionArn - ApiId: !GetAtt Api.Outputs.ApiId + ApiName: !GetAtt Api.Outputs.ApiName TemplateURL: monitoring-cf.yml LogBucket: diff --git a/apps/monitoring-cf.yml.j2 b/apps/monitoring-cf.yml.j2 index d140130fe..a814c9ff3 100644 --- a/apps/monitoring-cf.yml.j2 +++ b/apps/monitoring-cf.yml.j2 @@ -4,7 +4,7 @@ Parameters: StepFunctionArn: Type: String - ApiId: + ApiName: Type: String Resources: @@ -42,9 +42,9 @@ Resources: EvaluationPeriods: 1 Threshold: 0 Dimensions: - - Name: ApiId - Value: !Ref ApiId - MetricName: 5xx + - Name: ApiName + Value: !Ref ApiName + MetricName: 5XXError Namespace: AWS/ApiGateway Statistic: Sum TreatMissingData: notBreaching From f36aed6462a033ec56d88af8c1d4effe9ca837fc Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Wed, 31 Jan 2024 15:37:18 -0900 Subject: [PATCH 109/115] add DAR tags to S3 buckets in earthdata cloud deployments --- CHANGELOG.md | 5 +++++ apps/main-cf.yml.j2 | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f08f3c814..df8a53f85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [6.0.1] +### Added +- A `DAR` tag is now included in Earthdata Cloud deployments for each S3 bucket to communicate which contain objects + that required to be encrypted at rest. + ## [6.0.0] HyP3's monthly quota system has been replaced by a credits system. Previously, HyP3 provided each user with a certain number of jobs per month. Now, each job costs a particular number of credits, and users spend credits when they submit jobs. This release assigns every job a cost of 1 credit, but future releases will assign a different credit cost to each job type. Additionally, the main production deployment (`https://hyp3-api.asf.alaska.edu`) resets each user's balance to 1,000 credits each month, effectively granting each user 1,000 jobs per month. Therefore, users should not notice any difference when ordering jobs via ASF's On Demand service at . diff --git a/apps/main-cf.yml.j2 b/apps/main-cf.yml.j2 index 8ff11876e..ae5e6b35b 100644 --- a/apps/main-cf.yml.j2 +++ b/apps/main-cf.yml.j2 @@ -216,6 +216,11 @@ Resources: OwnershipControls: Rules: - ObjectOwnership: BucketOwnerEnforced + {% if security_environment == 'EDC' %} + Tags: + - Key: DAR + Value: YES + {% endif %} LogBucketPolicy: Type: AWS::S3::BucketPolicy @@ -273,6 +278,11 @@ Resources: OwnershipControls: Rules: - ObjectOwnership: BucketOwnerEnforced + {% if security_environment == 'EDC' %} + Tags: + - Key: DAR + Value: NO + {% endif %} {% if security_environment != 'JPL' %} BucketPolicy: From bccdb49edb7a3825a571bbe31a4a176eea4a5305 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Wed, 31 Jan 2024 16:08:13 -0900 Subject: [PATCH 110/115] move changelog entry under 6.0.0 --- CHANGELOG.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d825ac88b..6e1456bc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,6 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [6.0.1] - -### Fixed -- The `monitoring` module now successfully sends SNS notifications when the API encounters HTTP 5xx errors. Fixes [#2044](https://github.com/ASFHyP3/hyp3/issues/2044). - ## [6.0.0] HyP3's monthly quota system has been replaced by a credits system. Previously, HyP3 provided each user with a certain number of jobs per month. Now, each job costs a particular number of credits, and users spend credits when they submit jobs. This release assigns every job a cost of 1 credit, but future releases will assign a different credit cost to each job type. Additionally, the main production deployment (`https://hyp3-api.asf.alaska.edu`) resets each user's balance to 1,000 credits each month, effectively granting each user 1,000 jobs per month. Therefore, users should not notice any difference when ordering jobs via ASF's On Demand service at . @@ -19,6 +14,9 @@ HyP3's monthly quota system has been replaced by a credits system. Previously, H ### Changed - The `quota` attribute of the `user` object returned by the `/user` API endpoint has been replaced by a `remaining_credits` attribute, which represents the user's remaining credits. +### Fixed +- The `monitoring` module now successfully sends SNS notifications when the API encounters HTTP 5xx errors. Fixes [#2044](https://github.com/ASFHyP3/hyp3/issues/2044). + ## [5.0.4] ### Added - `INSAR_ISCE_BURST` jobs are now available in the azdwr-hyp3 deployment. From f8b62bab53c13e992e397d9ac772de21c2c6e29f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Feb 2024 19:12:12 +0000 Subject: [PATCH 111/115] Bump boto3 from 1.34.31 to 1.34.32 Bumps [boto3](https://github.com/boto/boto3) from 1.34.31 to 1.34.32. - [Release notes](https://github.com/boto/boto3/releases) - [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst) - [Commits](https://github.com/boto/boto3/compare/1.34.31...1.34.32) --- updated-dependencies: - dependency-name: boto3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- requirements-all.txt | 2 +- requirements-apps-disable-private-dns.txt | 2 +- requirements-apps-start-execution-manager.txt | 2 +- requirements-apps-start-execution-worker.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements-all.txt b/requirements-all.txt index 248769433..cb2e9f189 100644 --- a/requirements-all.txt +++ b/requirements-all.txt @@ -5,7 +5,7 @@ -r requirements-apps-start-execution-worker.txt -r requirements-apps-disable-private-dns.txt -r requirements-apps-update-db.txt -boto3==1.34.31 +boto3==1.34.32 jinja2==3.1.3 moto[dynamodb]==5.0.0 pytest==8.0.0 diff --git a/requirements-apps-disable-private-dns.txt b/requirements-apps-disable-private-dns.txt index 67ae4b2fc..aa2ff1534 100644 --- a/requirements-apps-disable-private-dns.txt +++ b/requirements-apps-disable-private-dns.txt @@ -1 +1 @@ -boto3==1.34.31 +boto3==1.34.32 diff --git a/requirements-apps-start-execution-manager.txt b/requirements-apps-start-execution-manager.txt index 99bff074b..a669566f2 100644 --- a/requirements-apps-start-execution-manager.txt +++ b/requirements-apps-start-execution-manager.txt @@ -1,3 +1,3 @@ -boto3==1.34.31 +boto3==1.34.32 ./lib/dynamo/ ./lib/lambda_logging/ diff --git a/requirements-apps-start-execution-worker.txt b/requirements-apps-start-execution-worker.txt index babdf3ad1..6301bd0a5 100644 --- a/requirements-apps-start-execution-worker.txt +++ b/requirements-apps-start-execution-worker.txt @@ -1,2 +1,2 @@ -boto3==1.34.31 +boto3==1.34.32 ./lib/lambda_logging/ From d19faa4c30c2cfde17dec7d00b3a0e82f92db381 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Thu, 1 Feb 2024 11:23:44 -0900 Subject: [PATCH 112/115] move changelog entry under 6.0.0 release --- CHANGELOG.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 204169c0a..08b425a09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,17 +4,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [6.0.1] -### Added -- A `DAR` tag is now included in Earthdata Cloud deployments for each S3 bucket to communicate which contain objects - that required to be encrypted at rest. - ## [6.0.0] HyP3's monthly quota system has been replaced by a credits system. Previously, HyP3 provided each user with a certain number of jobs per month. Now, each job costs a particular number of credits, and users spend credits when they submit jobs. This release assigns every job a cost of 1 credit, but future releases will assign a different credit cost to each job type. Additionally, the main production deployment (`https://hyp3-api.asf.alaska.edu`) resets each user's balance to 1,000 credits each month, effectively granting each user 1,000 jobs per month. Therefore, users should not notice any difference when ordering jobs via ASF's On Demand service at . ### Added - The `job` object returned by the `/jobs` API endpoints now includes a `credit_cost` attribute, which represents the job's cost in credits. +- A `DAR` tag is now included in Earthdata Cloud deployments for each S3 bucket to communicate which contain objects + that required to be encrypted at rest. ### Changed - The `quota` attribute of the `user` object returned by the `/user` API endpoint has been replaced by a `remaining_credits` attribute, which represents the user's remaining credits. From 78db59a2f1adca82c08a1b494024a064f22521da Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Thu, 1 Feb 2024 12:32:26 -0900 Subject: [PATCH 113/115] quota yes/no string to keep yaml parser from changing them to true/false --- apps/main-cf.yml.j2 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/main-cf.yml.j2 b/apps/main-cf.yml.j2 index 6e12c3194..1b79dbc8c 100644 --- a/apps/main-cf.yml.j2 +++ b/apps/main-cf.yml.j2 @@ -219,7 +219,7 @@ Resources: {% if security_environment == 'EDC' %} Tags: - Key: DAR - Value: YES + Value: "YES" {% endif %} LogBucketPolicy: @@ -281,7 +281,7 @@ Resources: {% if security_environment == 'EDC' %} Tags: - Key: DAR - Value: NO + Value: "NO" {% endif %} {% if security_environment != 'JPL' %} From 2fceb9aaef2bc8df9a2160fb8cdec14a64a0a66d Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Thu, 1 Feb 2024 14:42:59 -0900 Subject: [PATCH 114/115] remove non-functional cloudwatch alarm for api 5xx errors --- CHANGELOG.md | 4 ++-- apps/api/api-cf.yml.j2 | 3 --- apps/main-cf.yml.j2 | 1 - apps/monitoring-cf.yml.j2 | 21 --------------------- 4 files changed, 2 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08b425a09..ddb0f2a74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,8 +16,8 @@ HyP3's monthly quota system has been replaced by a credits system. Previously, H ### Changed - The `quota` attribute of the `user` object returned by the `/user` API endpoint has been replaced by a `remaining_credits` attribute, which represents the user's remaining credits. -### Fixed -- The `monitoring` module now successfully sends SNS notifications when the API encounters HTTP 5xx errors. Fixes [#2044](https://github.com/ASFHyP3/hyp3/issues/2044). +### Removed +- The non-functional CloudWatch alarm for API 5xx errors has been removed from the `monitoring` module. See [#2044](https://github.com/ASFHyP3/hyp3/issues/2044). ## [5.0.4] ### Added diff --git a/apps/api/api-cf.yml.j2 b/apps/api/api-cf.yml.j2 index 44e45f4d6..94f28cc8f 100644 --- a/apps/api/api-cf.yml.j2 +++ b/apps/api/api-cf.yml.j2 @@ -44,9 +44,6 @@ Outputs: Url: Value: {{ '!Sub "https://${RestApi}.execute-api.${AWS::Region}.amazonaws.com/${Stage}/ui"' if security_environment == 'EDC' else '!Sub "https://${CustomDomainName}/ui"' }} - ApiName: - Value: !GetAtt RestApi.RestApiId - Resources: RestApi: diff --git a/apps/main-cf.yml.j2 b/apps/main-cf.yml.j2 index 1b79dbc8c..df7d00fd5 100644 --- a/apps/main-cf.yml.j2 +++ b/apps/main-cf.yml.j2 @@ -197,7 +197,6 @@ Resources: Properties: Parameters: StepFunctionArn: !GetAtt StepFunction.Outputs.StepFunctionArn - ApiName: !GetAtt Api.Outputs.ApiName TemplateURL: monitoring-cf.yml LogBucket: diff --git a/apps/monitoring-cf.yml.j2 b/apps/monitoring-cf.yml.j2 index a814c9ff3..9e1a35a27 100644 --- a/apps/monitoring-cf.yml.j2 +++ b/apps/monitoring-cf.yml.j2 @@ -4,9 +4,6 @@ Parameters: StepFunctionArn: Type: String - ApiName: - Type: String - Resources: AlarmTopic: @@ -30,21 +27,3 @@ Resources: Statistic: Sum Unit: Count TreatMissingData: notBreaching - - ApiAlarm: - Type: AWS::CloudWatch::Alarm - Properties: - AlarmActions: - - !Ref AlarmTopic - AlarmDescription: Hyp3 api HTTP 500 errors - ComparisonOperator: GreaterThanThreshold - Period: 60 - EvaluationPeriods: 1 - Threshold: 0 - Dimensions: - - Name: ApiName - Value: !Ref ApiName - MetricName: 5XXError - Namespace: AWS/ApiGateway - Statistic: Sum - TreatMissingData: notBreaching From ec153c2624241486e7be179b635beb8174e2d6e4 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 2 Feb 2024 10:53:23 -0900 Subject: [PATCH 115/115] minor documentation tweaks prior to v6.0.0 release --- CHANGELOG.md | 2 +- docs/deployments/ASF-deployment.md | 2 +- docs/deployments/JPL-deployment.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddb0f2a74..7681319a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 HyP3's monthly quota system has been replaced by a credits system. Previously, HyP3 provided each user with a certain number of jobs per month. Now, each job costs a particular number of credits, and users spend credits when they submit jobs. This release assigns every job a cost of 1 credit, but future releases will assign a different credit cost to each job type. Additionally, the main production deployment (`https://hyp3-api.asf.alaska.edu`) resets each user's balance to 1,000 credits each month, effectively granting each user 1,000 jobs per month. Therefore, users should not notice any difference when ordering jobs via ASF's On Demand service at . ### Added -- The `job` object returned by the `/jobs` API endpoints now includes a `credit_cost` attribute, which represents the job's cost in credits. +- The `job` object returned by the `/jobs` API endpoint now includes a `credit_cost` attribute, which represents the job's cost in credits. - A `DAR` tag is now included in Earthdata Cloud deployments for each S3 bucket to communicate which contain objects that required to be encrypted at rest. diff --git a/docs/deployments/ASF-deployment.md b/docs/deployments/ASF-deployment.md index d42368428..11c74e6e4 100644 --- a/docs/deployments/ASF-deployment.md +++ b/docs/deployments/ASF-deployment.md @@ -94,5 +94,5 @@ aws cloudformation deploy \ CertificateArn= \ SecretArn= \ DefaultCreditsPerUser=0 \ - ResetCreditsMonthly=no + ResetCreditsMonthly=true ``` diff --git a/docs/deployments/JPL-deployment.md b/docs/deployments/JPL-deployment.md index 5d01da13f..eca192a3b 100644 --- a/docs/deployments/JPL-deployment.md +++ b/docs/deployments/JPL-deployment.md @@ -94,7 +94,7 @@ aws cloudformation deploy \ CertificateArn= \ SecretArn= \ DefaultCreditsPerUser=0 \ - ResetCreditsMonthly=no + ResetCreditsMonthly=true ``` ## 5. Post deployment