From 0951ced019b73ecacd956233595aa96f39c6f537 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Apr 2024 14:03:58 -0800 Subject: [PATCH 01/84] raise `UnapprovedUserError` where appropriate --- apps/api/src/hyp3_api/handlers.py | 7 ++++++- lib/dynamo/dynamo/jobs.py | 11 ++++++++++- lib/dynamo/dynamo/user.py | 33 +++++++++++++++++-------------- 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index fd873a67e..9bacb13a0 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -43,6 +43,8 @@ def post_jobs(body, user): try: body['jobs'] = dynamo.jobs.put_jobs(user, body['jobs'], dry_run=body.get('validate_only')) + except dynamo.user.UnapprovedUserError as e: + abort(problem_format(403, str(e))) except dynamo.jobs.InsufficientCreditsError as e: abort(problem_format(400, str(e))) return body @@ -79,7 +81,10 @@ def get_names_for_user(user): def get_user(user): - user_record = dynamo.user.get_or_create_user(user) + try: + user_record = dynamo.user.get_user(user) + except dynamo.user.UnapprovedUserError as e: + abort(problem_format(403, str(e))) 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 5048df66f..cdb72d8cf 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -30,7 +30,16 @@ 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_or_create_user(user_id) + user_record = dynamo.user.get_user(user_id) + + if user_record['application_status'] == 'PENDING': + raise dynamo.user.UnapprovedUserError(f'User {user_id} has a pending application, please try again later.') + elif user_record['application_status'] == 'REJECTED': + raise dynamo.user.UnapprovedUserError( + f'Unfortunately, the application for user {user_id} has been rejected.' + ' If you believe this was a mistake, please email ASF User Services at: uso@asf.alaska.edu' + ) + remaining_credits = user_record['remaining_credits'] priority_override = user_record.get('priority_override') diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index ee16b9eea..97eb9b604 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -12,34 +12,37 @@ class DatabaseConditionException(Exception): """Raised when a DynamoDB condition expression check fails.""" -def get_or_create_user(user_id: str) -> dict: +class UnapprovedUserError(Exception): + """Raised when the user is not approved for processing.""" + + +def get_user(user_id: str) -> dict: 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: - user = _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, + if user is None: + raise UnapprovedUserError( + # TODO: replace with registration form URL + f'User {user_id} has not yet applied for a monthly credit allotment.' + ' Please visit to submit your application.' ) - return user + + return _reset_credits_if_needed( + user=user, + default_credits=default_credits, + current_month=current_month, + users_table=users_table, + ) def _get_current_month() -> str: return datetime.now(tz=timezone.utc).strftime('%Y-%m') +# TODO: use this function somewhere or delete it 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 a8ff6fcaea086a375e36af5702ebfbbe0b10e0db Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Apr 2024 14:34:16 -0800 Subject: [PATCH 02/84] raise ValueError for invalid application status --- lib/dynamo/dynamo/jobs.py | 8 ++++++-- lib/dynamo/dynamo/user.py | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index cdb72d8cf..1e6f46810 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -9,6 +9,7 @@ from boto3.dynamodb.conditions import Attr, Key import dynamo.user +from dynamo.user import APPLICATION_APPROVED, APPLICATION_PENDING, APPLICATION_REJECTED from dynamo.util import DYNAMODB_RESOURCE, convert_floats_to_decimals, format_time, get_request_time_expression costs_file = Path(__file__).parent / 'costs.json' @@ -32,13 +33,16 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: user_record = dynamo.user.get_user(user_id) - if user_record['application_status'] == 'PENDING': + application_status = user_record['application_status'] + if application_status == APPLICATION_PENDING: raise dynamo.user.UnapprovedUserError(f'User {user_id} has a pending application, please try again later.') - elif user_record['application_status'] == 'REJECTED': + elif application_status == APPLICATION_REJECTED: raise dynamo.user.UnapprovedUserError( f'Unfortunately, the application for user {user_id} has been rejected.' ' If you believe this was a mistake, please email ASF User Services at: uso@asf.alaska.edu' ) + elif application_status != APPLICATION_APPROVED: + raise ValueError(f'User {user_id} has an invalid application status: {application_status}') remaining_credits = user_record['remaining_credits'] priority_override = user_record.get('priority_override') diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 97eb9b604..24c415b5e 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -7,6 +7,10 @@ from dynamo.util import DYNAMODB_RESOURCE +APPLICATION_APPROVED = 'APPROVED' +APPLICATION_PENDING = 'PENDING' +APPLICATION_REJECTED = 'REJECTED' + class DatabaseConditionException(Exception): """Raised when a DynamoDB condition expression check fails.""" From d5fec6ad9d845bcce79703c748cb509485cc8d90 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Apr 2024 14:52:56 -0800 Subject: [PATCH 03/84] add application_status to /user response --- apps/api/src/hyp3_api/handlers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 9bacb13a0..0181bcfda 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -87,7 +87,8 @@ def get_user(user): abort(problem_format(403, str(e))) return { 'user_id': user, + 'application_status': user_record['application_status'], '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) + 'job_names': get_names_for_user(user), } From cc45b5a65a15fe6c647127cce6cf2ba5ddc94ce4 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Apr 2024 15:03:43 -0800 Subject: [PATCH 04/84] start updating create_user and _reset_credits_if_needed --- lib/dynamo/dynamo/user.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 24c415b5e..b8efe0cc0 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -46,19 +46,26 @@ def _get_current_month() -> str: return datetime.now(tz=timezone.utc).strftime('%Y-%m') -# TODO: use this function somewhere or delete it -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} +# TODO: call this function for the user registration endpoint +def create_user(user_id: str, users_table) -> dict: + user = {'user_id': user_id, 'remaining_credits': Decimal(0), 'month_of_last_credits_reset': '0'} 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': + # TODO: replace this with a different exception and convert it to a client error + # for the user registration endpoint raise DatabaseConditionException(f'Failed to create user {user_id}') raise return user def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> dict: + # TODO: + # if application_status is APPROVED: + # take the current approach + # elif application_status is PENDING or REJECTED: + # set remaining_credits and month of last reset to 0 (like in create_user) if ( os.environ['RESET_CREDITS_MONTHLY'] == 'true' and user['month_of_last_credits_reset'] < current_month # noqa: W503 From cb75eab8f63cdd682d4fa1908282404661d5d447 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 25 Apr 2024 15:06:23 -0800 Subject: [PATCH 05/84] create user with application pending --- lib/dynamo/dynamo/user.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index b8efe0cc0..8d704790f 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -48,7 +48,12 @@ def _get_current_month() -> str: # TODO: call this function for the user registration endpoint def create_user(user_id: str, users_table) -> dict: - user = {'user_id': user_id, 'remaining_credits': Decimal(0), 'month_of_last_credits_reset': '0'} + user = { + 'user_id': user_id, + 'remaining_credits': Decimal(0), + 'month_of_last_credits_reset': '0', + 'application_status': APPLICATION_PENDING, + } try: users_table.put_item(Item=user, ConditionExpression='attribute_not_exists(user_id)') except botocore.exceptions.ClientError as e: From 5fef814353b90ad8991ff534c652872b0a4eb53e Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 26 Apr 2024 09:26:43 -0800 Subject: [PATCH 06/84] reset credits to 0 for unapproved users --- lib/dynamo/dynamo/user.py | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 8d704790f..994a4526b 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -66,13 +66,9 @@ def create_user(user_id: str, users_table) -> dict: def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> dict: - # TODO: - # if application_status is APPROVED: - # take the current approach - # elif application_status is PENDING or REJECTED: - # set remaining_credits and month of last reset to 0 (like in create_user) if ( os.environ['RESET_CREDITS_MONTHLY'] == 'true' + and user['application_status'] == APPLICATION_APPROVED and user['month_of_last_credits_reset'] < current_month # noqa: W503 and user['remaining_credits'] is not None # noqa: W503 ): @@ -81,16 +77,39 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month try: users_table.put_item( Item=user, - ConditionExpression='month_of_last_credits_reset < :current_month' - ' AND attribute_type(remaining_credits, :number)', + ConditionExpression=( + 'application_status = :approved' + ' month_of_last_credits_reset < :current_month' + ' AND attribute_type(remaining_credits, :number)' + ), ExpressionAttributeValues={ + ':approved': APPLICATION_APPROVED, ':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["user_id"]}') + raise DatabaseConditionException( + f'Failed to perform monthly credit reset for approved user {user["user_id"]}' + ) + raise + elif user['application_status'] != APPLICATION_APPROVED: + # TODO should we also check RESET_CREDITS_MONTHLY for this case? should we perhaps get rid of that + # variable for now since hyp3-enterprise-test is the only deployment that sets it false? + user['month_of_last_credits_reset'] = '0' + user['remaining_credits'] = Decimal(0) + try: + users_table.put_item( + Item=user, + ConditionExpression='NOT (application_status = :approved)', + ExpressionAttributeValues={':approved': APPLICATION_APPROVED}, + ) + except botocore.exceptions.ClientError as e: + if e.response['Error']['Code'] == 'ConditionalCheckFailedException': + raise DatabaseConditionException( + f'Failed to perform monthly credit reset for unapproved user {user["user_id"]}' + ) raise return user From 160896c218e85b6a7ff9d62d1d12652c67372d5e Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 26 Apr 2024 10:45:14 -0800 Subject: [PATCH 07/84] Add /register endpoint --- apps/api/src/hyp3_api/handlers.py | 11 +++++++++++ apps/api/src/hyp3_api/routes.py | 7 +++++++ lib/dynamo/dynamo/user.py | 23 ++++++++++++++--------- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 0181bcfda..3eeefe5ef 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -31,6 +31,17 @@ def is_uuid(val): return True +def register(body, user): + print(body) + + try: + payload = dynamo.user.create_user(user, body) + except dynamo.user.UserAlreadyExistsError as e: + # TODO is there a more specific status code to use here? + abort(problem_format(400, str(e))) + return payload + + def post_jobs(body, user): print(body) diff --git a/apps/api/src/hyp3_api/routes.py b/apps/api/src/hyp3_api/routes.py index 23ace2bad..777f28353 100644 --- a/apps/api/src/hyp3_api/routes.py +++ b/apps/api/src/hyp3_api/routes.py @@ -148,6 +148,13 @@ def jobs_get_by_job_id(job_id): return jsonify(handlers.get_job_by_id(job_id)) +@app.route('/register', methods=['POST']) +@openapi +def register_post(): + # TODO: add to API spec + return jsonify(handlers.register(request.get_json(), g.user)) + + @app.route('/user', methods=['GET']) @openapi def user_get(): diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 994a4526b..f8aecc93d 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -16,6 +16,10 @@ class DatabaseConditionException(Exception): """Raised when a DynamoDB condition expression check fails.""" +class UserAlreadyExistsError(Exception): + """Raised when an existing user attempts to re-register.""" + + class UnapprovedUserError(Exception): """Raised when the user is not approved for processing.""" @@ -46,23 +50,24 @@ def _get_current_month() -> str: return datetime.now(tz=timezone.utc).strftime('%Y-%m') -# TODO: call this function for the user registration endpoint -def create_user(user_id: str, users_table) -> dict: - user = { +def create_user(user_id: str, body: dict) -> dict: + users_table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) + body.update({ 'user_id': user_id, 'remaining_credits': Decimal(0), 'month_of_last_credits_reset': '0', 'application_status': APPLICATION_PENDING, - } + }) try: - users_table.put_item(Item=user, ConditionExpression='attribute_not_exists(user_id)') + users_table.put_item(Item=body, ConditionExpression='attribute_not_exists(user_id)') except botocore.exceptions.ClientError as e: if e.response['Error']['Code'] == 'ConditionalCheckFailedException': - # TODO: replace this with a different exception and convert it to a client error - # for the user registration endpoint - raise DatabaseConditionException(f'Failed to create user {user_id}') + raise UserAlreadyExistsError(f'User {user_id} has already submitted an application.') raise - return user + # TODO: should we perhaps return the entire user record here, + # minus any fields that we don't want them to see, e.g. priority? + # And same for the GET /user endpoint? + return {} def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> dict: From e7759413c8d1b055d9aa5cac99498563eb9532fb Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 26 Apr 2024 10:50:54 -0800 Subject: [PATCH 08/84] change /register to POST /user --- apps/api/src/hyp3_api/handlers.py | 22 ++++++++--------- apps/api/src/hyp3_api/routes.py | 6 ++--- lib/dynamo/dynamo/user.py | 40 +++++++++++++++---------------- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 3eeefe5ef..d364cdea1 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -31,17 +31,6 @@ def is_uuid(val): return True -def register(body, user): - print(body) - - try: - payload = dynamo.user.create_user(user, body) - except dynamo.user.UserAlreadyExistsError as e: - # TODO is there a more specific status code to use here? - abort(problem_format(400, str(e))) - return payload - - def post_jobs(body, user): print(body) @@ -91,6 +80,17 @@ def get_names_for_user(user): return sorted(list(names)) +def post_user(body, user): + print(body) + + try: + payload = dynamo.user.create_user(user, body) + except dynamo.user.UserAlreadyExistsError as e: + # TODO is there a more specific status code to use here? + abort(problem_format(400, str(e))) + return payload + + def get_user(user): try: user_record = dynamo.user.get_user(user) diff --git a/apps/api/src/hyp3_api/routes.py b/apps/api/src/hyp3_api/routes.py index 777f28353..2209b9a37 100644 --- a/apps/api/src/hyp3_api/routes.py +++ b/apps/api/src/hyp3_api/routes.py @@ -148,11 +148,11 @@ def jobs_get_by_job_id(job_id): return jsonify(handlers.get_job_by_id(job_id)) -@app.route('/register', methods=['POST']) +@app.route('/user', methods=['POST']) @openapi -def register_post(): +def user_post(): # TODO: add to API spec - return jsonify(handlers.register(request.get_json(), g.user)) + return jsonify(handlers.post_user(request.get_json(), g.user)) @app.route('/user', methods=['GET']) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index f8aecc93d..ff51b908f 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -24,6 +24,26 @@ class UnapprovedUserError(Exception): """Raised when the user is not approved for processing.""" +def create_user(user_id: str, body: dict) -> dict: + users_table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) + body.update({ + 'user_id': user_id, + 'remaining_credits': Decimal(0), + 'month_of_last_credits_reset': '0', + 'application_status': APPLICATION_PENDING, + }) + try: + users_table.put_item(Item=body, ConditionExpression='attribute_not_exists(user_id)') + except botocore.exceptions.ClientError as e: + if e.response['Error']['Code'] == 'ConditionalCheckFailedException': + raise UserAlreadyExistsError(f'User {user_id} has already submitted an application.') + raise + # TODO: should we perhaps return the entire user record here, + # minus any fields that we don't want them to see, e.g. priority? + # And same for the GET /user endpoint? + return {} + + def get_user(user_id: str) -> dict: current_month = _get_current_month() default_credits = Decimal(os.environ['DEFAULT_CREDITS_PER_USER']) @@ -50,26 +70,6 @@ def _get_current_month() -> str: return datetime.now(tz=timezone.utc).strftime('%Y-%m') -def create_user(user_id: str, body: dict) -> dict: - users_table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) - body.update({ - 'user_id': user_id, - 'remaining_credits': Decimal(0), - 'month_of_last_credits_reset': '0', - 'application_status': APPLICATION_PENDING, - }) - try: - users_table.put_item(Item=body, ConditionExpression='attribute_not_exists(user_id)') - except botocore.exceptions.ClientError as e: - if e.response['Error']['Code'] == 'ConditionalCheckFailedException': - raise UserAlreadyExistsError(f'User {user_id} has already submitted an application.') - raise - # TODO: should we perhaps return the entire user record here, - # minus any fields that we don't want them to see, e.g. priority? - # And same for the GET /user endpoint? - return {} - - def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> dict: if ( os.environ['RESET_CREDITS_MONTHLY'] == 'true' From bf39a9dd1d4a7a80be857678734a1c62c694da07 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 26 Apr 2024 11:18:28 -0800 Subject: [PATCH 09/84] add whitelisting sandbox deployment --- .../workflows/deploy-whitelisting-sandbox.yml | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 .github/workflows/deploy-whitelisting-sandbox.yml diff --git a/.github/workflows/deploy-whitelisting-sandbox.yml b/.github/workflows/deploy-whitelisting-sandbox.yml new file mode 100644 index 000000000..7f01b5358 --- /dev/null +++ b/.github/workflows/deploy-whitelisting-sandbox.yml @@ -0,0 +1,83 @@ +name: Deploy Whitelisting Sandbox Stack to AWS + +on: + push: + branches: + - whitelisting-sandbox + +concurrency: ${{ github.workflow }}-${{ github.ref }} + +jobs: + deploy: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - environment: hyp3-whitelisting-sandbox + domain: hyp3-whitelisting-sandbox.asf.alaska.edu + template_bucket: cf-templates-1hz9ldhhl4ahu-us-west-2 + image_tag: test + product_lifetime_in_days: 14 + default_credits_per_user: 10 + reset_credits_monthly: true + cost_profile: EDC + deploy_ref: refs/heads/whitelisting-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.2 + + - 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 }} + DEFAULT_CREDITS_PER_USER: ${{ matrix.default_credits_per_user }} + RESET_CREDITS_MONTHLY: ${{ matrix.reset_credits_monthly }} + COST_PROFILE: ${{ matrix.cost_profile }} + 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 8a671c6f547462f17c55662736e686ce0e7b1caa Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 29 Apr 2024 19:30:02 +0000 Subject: [PATCH 10/84] Bump moto[dynamodb] from 5.0.5 to 5.0.6 Bumps [moto[dynamodb]](https://github.com/getmoto/moto) from 5.0.5 to 5.0.6. - [Release notes](https://github.com/getmoto/moto/releases) - [Changelog](https://github.com/getmoto/moto/blob/master/CHANGELOG.md) - [Commits](https://github.com/getmoto/moto/compare/5.0.5...5.0.6) --- updated-dependencies: - dependency-name: moto[dynamodb] dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- requirements-all.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-all.txt b/requirements-all.txt index 6bb9a7f51..0e1de3b06 100644 --- a/requirements-all.txt +++ b/requirements-all.txt @@ -7,7 +7,7 @@ -r requirements-apps-update-db.txt boto3==1.34.87 jinja2==3.1.3 -moto[dynamodb]==5.0.5 +moto[dynamodb]==5.0.6 pytest==8.1.1 PyYAML==6.0.1 responses==0.25.0 From 7da015b769dcf051b4ff0ecd58e78af6aa67ae15 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 29 Apr 2024 14:30:26 -0800 Subject: [PATCH 11/84] Add `POST /user` to API spec --- .../src/hyp3_api/api-spec/openapi-spec.yml.j2 | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) 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 309b2d699..94ff466f9 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 @@ -104,6 +104,22 @@ paths: $ref: "#/components/schemas/job" /user: + post: + description: Submit an application for processing approval. + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/post_user_body" + required: true + responses: + "200": + description: 200 response + content: + application/json: + schema: + $ref: "#/components/schemas/user" + get: description: Get information about the logged in user. responses: @@ -172,6 +188,16 @@ components: next: $ref: "#/components/schemas/next_url" + post_user_body: + description: Application for processing approval. + type: object + required: + - use_case + additionalProperties: false + properties: + use_case: + type: string + user: description: Information about a user type: object From 2dcc6231c71ac655aeef924110c0d7b83c1675bb Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 29 Apr 2024 15:01:41 -0800 Subject: [PATCH 12/84] Fix broken condition expression --- 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 ff51b908f..c185d735a 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -84,7 +84,7 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month Item=user, ConditionExpression=( 'application_status = :approved' - ' month_of_last_credits_reset < :current_month' + ' AND month_of_last_credits_reset < :current_month' ' AND attribute_type(remaining_credits, :number)' ), ExpressionAttributeValues={ From 8d84ec5b1dcb2336654a2f90c65a8500056ccda1 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 30 Apr 2024 15:52:25 -0800 Subject: [PATCH 13/84] Add fourth application_status value: `NOT STARTED` --- .../src/hyp3_api/api-spec/openapi-spec.yml.j2 | 8 +- apps/api/src/hyp3_api/handlers.py | 12 +- apps/api/src/hyp3_api/routes.py | 7 +- lib/dynamo/dynamo/jobs.py | 10 +- lib/dynamo/dynamo/user.py | 103 ++++++++++++------ 5 files changed, 90 insertions(+), 50 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 94ff466f9..434017f0c 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 @@ -104,13 +104,13 @@ paths: $ref: "#/components/schemas/job" /user: - post: - description: Submit an application for processing approval. + patch: + description: Submit or update an application for processing approval. requestBody: content: application/json: schema: - $ref: "#/components/schemas/post_user_body" + $ref: "#/components/schemas/patch_user_body" required: true responses: "200": @@ -188,7 +188,7 @@ components: next: $ref: "#/components/schemas/next_url" - post_user_body: + patch_user_body: description: Application for processing approval. type: object required: diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index fb10dcb75..5078c260d 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -69,18 +69,18 @@ def get_names_for_user(user): return sorted(list(names)) -def post_user(body, user): +def patch_user(body, user): print(body) - try: - payload = dynamo.user.create_user(user, body) - except dynamo.user.UserAlreadyExistsError as e: - # TODO is there a more specific status code to use here? - abort(problem_format(400, str(e))) + payload = dynamo.user.update_user(user, body) + except dynamo.user.CannotUpdateUserError as e: + # TODO is this the appropriate status code? + abort(problem_format(403, str(e))) return payload def get_user(user): + # TODO: update so as not to raise 403 try: user_record = dynamo.user.get_user(user) except dynamo.user.UnapprovedUserError as e: diff --git a/apps/api/src/hyp3_api/routes.py b/apps/api/src/hyp3_api/routes.py index 2209b9a37..a6a50917c 100644 --- a/apps/api/src/hyp3_api/routes.py +++ b/apps/api/src/hyp3_api/routes.py @@ -148,11 +148,10 @@ def jobs_get_by_job_id(job_id): return jsonify(handlers.get_job_by_id(job_id)) -@app.route('/user', methods=['POST']) +@app.route('/user', methods=['PATCH']) @openapi -def user_post(): - # TODO: add to API spec - return jsonify(handlers.post_user(request.get_json(), g.user)) +def user_patch(): + return jsonify(handlers.patch_user(request.get_json(), g.user)) @app.route('/user', methods=['GET']) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 1e6f46810..06041e418 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -9,7 +9,7 @@ from boto3.dynamodb.conditions import Attr, Key import dynamo.user -from dynamo.user import APPLICATION_APPROVED, APPLICATION_PENDING, APPLICATION_REJECTED +from dynamo.user import APPLICATION_APPROVED, APPLICATION_NOT_STARTED, APPLICATION_PENDING, APPLICATION_REJECTED from dynamo.util import DYNAMODB_RESOURCE, convert_floats_to_decimals, format_time, get_request_time_expression costs_file = Path(__file__).parent / 'costs.json' @@ -31,9 +31,15 @@ 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) + user_record = dynamo.user.get_or_create_user(user_id) application_status = user_record['application_status'] + if application_status == APPLICATION_NOT_STARTED: + # TODO replace with URL to the application form for the given deployment + raise dynamo.user.UnapprovedUserError( + f'User {user_id} has not yet applied for a monthly credit allotment.' + ' Please visit to submit your application.' + ) if application_status == APPLICATION_PENDING: raise dynamo.user.UnapprovedUserError(f'User {user_id} has a pending application, please try again later.') elif application_status == APPLICATION_REJECTED: diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index c185d735a..7e3365d64 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -7,69 +7,104 @@ from dynamo.util import DYNAMODB_RESOURCE -APPLICATION_APPROVED = 'APPROVED' +APPLICATION_NOT_STARTED = 'NOT STARTED' APPLICATION_PENDING = 'PENDING' +APPLICATION_APPROVED = 'APPROVED' APPLICATION_REJECTED = 'REJECTED' class DatabaseConditionException(Exception): """Raised when a DynamoDB condition expression check fails.""" - -class UserAlreadyExistsError(Exception): - """Raised when an existing user attempts to re-register.""" +# TODO: should the following two exception types just be based on the HTTP error code that should be raised, +# e.g. 403Error? class UnapprovedUserError(Exception): """Raised when the user is not approved for processing.""" -def create_user(user_id: str, body: dict) -> dict: - users_table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) - body.update({ - 'user_id': user_id, - 'remaining_credits': Decimal(0), - 'month_of_last_credits_reset': '0', - 'application_status': APPLICATION_PENDING, - }) - try: - users_table.put_item(Item=body, ConditionExpression='attribute_not_exists(user_id)') - except botocore.exceptions.ClientError as e: - if e.response['Error']['Code'] == 'ConditionalCheckFailedException': - raise UserAlreadyExistsError(f'User {user_id} has already submitted an application.') - raise - # TODO: should we perhaps return the entire user record here, - # minus any fields that we don't want them to see, e.g. priority? - # And same for the GET /user endpoint? +class CannotUpdateUserError(Exception): + """Raised when the user record cannot be updated.""" + + +def update_user(user_id: str, body: dict) -> dict: + # TODO also set derived EDL fields + user = get_or_create_user(user_id) + application_status = user['application_status'] + if application_status in (APPLICATION_NOT_STARTED, APPLICATION_PENDING): + users_table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) + try: + users_table.update_item( + Key={'user_id': user_id}, + UpdateExpression='SET use_case = :use_case', + ConditionExpression='application_status IN (:not_started, :pending)', + ExpressionAttributeValues={ + ':use_case': body['use_case'], + ':not_started': APPLICATION_NOT_STARTED, + ':pending': APPLICATION_PENDING + }, + ) + except botocore.exceptions.ClientError as e: + if e.response['Error']['Code'] == 'ConditionalCheckFailedException': + raise DatabaseConditionException(f'Failed to update record for user {user_id}') + raise + elif application_status == APPLICATION_REJECTED: + raise CannotUpdateUserError( + f'Unfortunately, the application for user {user_id} has been rejected.' + ' If you believe this was a mistake, please email ASF User Services at: uso@asf.alaska.edu' + ) + elif application_status == APPLICATION_APPROVED: + raise CannotUpdateUserError(f'The application for user {user_id} has already been approved.') + else: + raise ValueError(f'User {user_id} has an invalid application status: {application_status}') + # TODO return updated user record return {} -def get_user(user_id: str) -> dict: +def get_or_create_user(user_id: str) -> dict: 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 None: - raise UnapprovedUserError( - # TODO: replace with registration form URL - f'User {user_id} has not yet applied for a monthly credit allotment.' - ' Please visit to submit your application.' + if user is not None: + user = _reset_credits_if_needed( + user=user, + default_credits=default_credits, + current_month=current_month, + users_table=users_table, ) - - return _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, + users_table=users_table, + ) + return user def _get_current_month() -> str: return datetime.now(tz=timezone.utc).strftime('%Y-%m') +def _create_user(user_id: str, users_table) -> dict: + # TODO rename month_of_last_credits_reset -> month_of_last_credit_reset + user = { + 'user_id': user_id, + 'remaining_credits': Decimal(0), + 'month_of_last_credits_reset': '0', + 'application_status': APPLICATION_NOT_STARTED + } + 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 + + def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> dict: if ( os.environ['RESET_CREDITS_MONTHLY'] == 'true' From 0cddb9547d9f0da6432f42b58a574deaade0abbd Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Wed, 1 May 2024 10:13:20 -0800 Subject: [PATCH 14/84] update get_user --- apps/api/src/hyp3_api/handlers.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 5078c260d..6eeb7757c 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -80,11 +80,7 @@ def patch_user(body, user): def get_user(user): - # TODO: update so as not to raise 403 - try: - user_record = dynamo.user.get_user(user) - except dynamo.user.UnapprovedUserError as e: - abort(problem_format(403, str(e))) + user_record = dynamo.user.get_or_create_user(user) return { 'user_id': user, 'application_status': user_record['application_status'], From 9373f02e02ea0f39071e9b29d48b7d94bb86cd0d Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Wed, 1 May 2024 10:51:53 -0800 Subject: [PATCH 15/84] add a TODO and set app status to pending --- lib/dynamo/dynamo/jobs.py | 1 + lib/dynamo/dynamo/user.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 06041e418..8e234f5ea 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -33,6 +33,7 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: user_record = dynamo.user.get_or_create_user(user_id) + # TODO factor out helper func application_status = user_record['application_status'] if application_status == APPLICATION_NOT_STARTED: # TODO replace with URL to the application form for the given deployment diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 7e3365d64..af47c898b 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -37,7 +37,7 @@ def update_user(user_id: str, body: dict) -> dict: try: users_table.update_item( Key={'user_id': user_id}, - UpdateExpression='SET use_case = :use_case', + UpdateExpression='SET use_case = :use_case, application_status = :pending', ConditionExpression='application_status IN (:not_started, :pending)', ExpressionAttributeValues={ ':use_case': body['use_case'], From 638b5748c0aa903663aad160959cc834ac51545c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 1 May 2024 19:40:01 +0000 Subject: [PATCH 16/84] Bump jsonschema from 4.21.1 to 4.22.0 Bumps [jsonschema](https://github.com/python-jsonschema/jsonschema) from 4.21.1 to 4.22.0. - [Release notes](https://github.com/python-jsonschema/jsonschema/releases) - [Changelog](https://github.com/python-jsonschema/jsonschema/blob/main/CHANGELOG.rst) - [Commits](https://github.com/python-jsonschema/jsonschema/compare/v4.21.1...v4.22.0) --- updated-dependencies: - dependency-name: jsonschema dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- requirements-apps-api.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-apps-api.txt b/requirements-apps-api.txt index ef089808a..ff4dbab56 100644 --- a/requirements-apps-api.txt +++ b/requirements-apps-api.txt @@ -1,6 +1,6 @@ flask==2.2.5 Flask-Cors==4.0.0 -jsonschema==4.21.1 +jsonschema==4.22.0 openapi-core==0.19.1 prance==23.6.21.0 PyJWT==2.8.0 From 6e6db6777ebab1b067fb9049e2f79692d3e4461b Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 2 May 2024 09:06:45 -0800 Subject: [PATCH 17/84] return nothing from PATCH /user --- apps/api/src/hyp3_api/api-spec/openapi-spec.yml.j2 | 4 ---- apps/api/src/hyp3_api/handlers.py | 5 ++--- apps/api/src/hyp3_api/routes.py | 2 +- lib/dynamo/dynamo/user.py | 4 +--- 4 files changed, 4 insertions(+), 11 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 434017f0c..2ccf0c015 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 @@ -115,10 +115,6 @@ paths: responses: "200": description: 200 response - content: - application/json: - schema: - $ref: "#/components/schemas/user" get: description: Get information about the logged in user. diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 6eeb7757c..54ef73c12 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -69,14 +69,13 @@ def get_names_for_user(user): return sorted(list(names)) -def patch_user(body, user): +def patch_user(body: dict, user: str) -> None: print(body) try: - payload = dynamo.user.update_user(user, body) + dynamo.user.update_user(user, body) except dynamo.user.CannotUpdateUserError as e: # TODO is this the appropriate status code? abort(problem_format(403, str(e))) - return payload def get_user(user): diff --git a/apps/api/src/hyp3_api/routes.py b/apps/api/src/hyp3_api/routes.py index a6a50917c..785eb770b 100644 --- a/apps/api/src/hyp3_api/routes.py +++ b/apps/api/src/hyp3_api/routes.py @@ -151,7 +151,7 @@ def jobs_get_by_job_id(job_id): @app.route('/user', methods=['PATCH']) @openapi def user_patch(): - return jsonify(handlers.patch_user(request.get_json(), g.user)) + handlers.patch_user(request.get_json(), g.user) @app.route('/user', methods=['GET']) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index af47c898b..6e28a68ef 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -28,7 +28,7 @@ class CannotUpdateUserError(Exception): """Raised when the user record cannot be updated.""" -def update_user(user_id: str, body: dict) -> dict: +def update_user(user_id: str, body: dict) -> None: # TODO also set derived EDL fields user = get_or_create_user(user_id) application_status = user['application_status'] @@ -58,8 +58,6 @@ def update_user(user_id: str, body: dict) -> dict: raise CannotUpdateUserError(f'The application for user {user_id} has already been approved.') else: raise ValueError(f'User {user_id} has an invalid application status: {application_status}') - # TODO return updated user record - return {} def get_or_create_user(user_id: str) -> dict: From 45a2739c0fa7a7ca8acf41520bbc3bbdef0410f6 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 2 May 2024 09:24:54 -0800 Subject: [PATCH 18/84] return successful response from PATCH /user --- apps/api/src/hyp3_api/routes.py | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/api/src/hyp3_api/routes.py b/apps/api/src/hyp3_api/routes.py index 785eb770b..bd121b685 100644 --- a/apps/api/src/hyp3_api/routes.py +++ b/apps/api/src/hyp3_api/routes.py @@ -152,6 +152,7 @@ def jobs_get_by_job_id(job_id): @openapi def user_patch(): handlers.patch_user(request.get_json(), g.user) + return jsonify(success=True) @app.route('/user', methods=['GET']) From 08b9ce169cebec580a01f38646b40661ecceff5a Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 2 May 2024 09:36:48 -0800 Subject: [PATCH 19/84] expose use_case via GET /user --- apps/api/src/hyp3_api/handlers.py | 1 + apps/api/src/hyp3_api/routes.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 54ef73c12..9b24d8a89 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -83,6 +83,7 @@ def get_user(user): return { 'user_id': user, 'application_status': user_record['application_status'], + 'use_case': user_record['use_case'], '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), diff --git a/apps/api/src/hyp3_api/routes.py b/apps/api/src/hyp3_api/routes.py index bd121b685..97f3d4c27 100644 --- a/apps/api/src/hyp3_api/routes.py +++ b/apps/api/src/hyp3_api/routes.py @@ -152,7 +152,7 @@ def jobs_get_by_job_id(job_id): @openapi def user_patch(): handlers.patch_user(request.get_json(), g.user) - return jsonify(success=True) + return {} @app.route('/user', methods=['GET']) From 86d9e5decf6e3ed003c939bf23b35d70b2996432 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 2 May 2024 09:43:08 -0800 Subject: [PATCH 20/84] allow for no use_case --- 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 9b24d8a89..48c95ce38 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -83,7 +83,7 @@ def get_user(user): return { 'user_id': user, 'application_status': user_record['application_status'], - 'use_case': user_record['use_case'], + 'use_case': user_record.get('use_case'), '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 e8c482473ee7afd4c41ba1d72f358e23c97703c9 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 2 May 2024 10:20:04 -0800 Subject: [PATCH 21/84] refactor exceptions --- apps/api/src/hyp3_api/handlers.py | 4 +-- lib/dynamo/dynamo/jobs.py | 44 +++++++++++++++++++------------ lib/dynamo/dynamo/user.py | 15 +++-------- 3 files changed, 33 insertions(+), 30 deletions(-) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 48c95ce38..9b2d367ed 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -32,7 +32,7 @@ def post_jobs(body, user): try: body['jobs'] = dynamo.jobs.put_jobs(user, body['jobs'], dry_run=body.get('validate_only')) - except dynamo.user.UnapprovedUserError as e: + except dynamo.jobs.UnapprovedUserError as e: abort(problem_format(403, str(e))) except dynamo.jobs.InsufficientCreditsError as e: abort(problem_format(400, str(e))) @@ -73,7 +73,7 @@ def patch_user(body: dict, user: str) -> None: print(body) try: dynamo.user.update_user(user, body) - except dynamo.user.CannotUpdateUserError as e: + except dynamo.user.ApplicationClosedError as e: # TODO is this the appropriate status code? abort(problem_format(403, str(e))) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 8e234f5ea..2e646c04d 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -23,6 +23,10 @@ DEFAULT_PARAMS_BY_JOB_TYPE = {} +class UnapprovedUserError(Exception): + """Raised when the user is not approved for processing.""" + + class InsufficientCreditsError(Exception): """Raised when trying to submit jobs whose total cost exceeds the user's remaining credits.""" @@ -33,23 +37,7 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: user_record = dynamo.user.get_or_create_user(user_id) - # TODO factor out helper func - application_status = user_record['application_status'] - if application_status == APPLICATION_NOT_STARTED: - # TODO replace with URL to the application form for the given deployment - raise dynamo.user.UnapprovedUserError( - f'User {user_id} has not yet applied for a monthly credit allotment.' - ' Please visit to submit your application.' - ) - if application_status == APPLICATION_PENDING: - raise dynamo.user.UnapprovedUserError(f'User {user_id} has a pending application, please try again later.') - elif application_status == APPLICATION_REJECTED: - raise dynamo.user.UnapprovedUserError( - f'Unfortunately, the application for user {user_id} has been rejected.' - ' If you believe this was a mistake, please email ASF User Services at: uso@asf.alaska.edu' - ) - elif application_status != APPLICATION_APPROVED: - raise ValueError(f'User {user_id} has an invalid application status: {application_status}') + _raise_for_application_status(user_record['application_status'], user_record['user_id']) remaining_credits = user_record['remaining_credits'] priority_override = user_record.get('priority_override') @@ -84,6 +72,28 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: return prepared_jobs +def _raise_for_application_status(application_status: str, user_id: str) -> None: + if application_status == APPLICATION_NOT_STARTED: + # TODO replace with URL to the application form for the given deployment + raise UnapprovedUserError( + f'User {user_id} has not yet applied for a monthly credit allotment.' + ' Please visit to submit your application.' + ) + if application_status == APPLICATION_PENDING: + raise UnapprovedUserError( + f'User {user_id} has a pending application, please try again later.' + ) + if application_status == APPLICATION_REJECTED: + raise UnapprovedUserError( + f'Unfortunately, the application for user {user_id} has been rejected.' + ' If you believe this was a mistake, please email ASF User Services at: uso@asf.alaska.edu' + ) + if application_status != APPLICATION_APPROVED: + raise ValueError( + f'User {user_id} has an invalid application status: {application_status}' + ) + + def _prepare_job_for_database( job: dict, user_id: str, diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 6e28a68ef..5a58aafdb 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -16,16 +16,9 @@ class DatabaseConditionException(Exception): """Raised when a DynamoDB condition expression check fails.""" -# TODO: should the following two exception types just be based on the HTTP error code that should be raised, -# e.g. 403Error? - -class UnapprovedUserError(Exception): - """Raised when the user is not approved for processing.""" - - -class CannotUpdateUserError(Exception): - """Raised when the user record cannot be updated.""" +class ApplicationClosedError(Exception): + """Raised when the user attempts to update an application that has already been approved or rejected.""" def update_user(user_id: str, body: dict) -> None: @@ -50,12 +43,12 @@ def update_user(user_id: str, body: dict) -> None: raise DatabaseConditionException(f'Failed to update record for user {user_id}') raise elif application_status == APPLICATION_REJECTED: - raise CannotUpdateUserError( + raise ApplicationClosedError( f'Unfortunately, the application for user {user_id} has been rejected.' ' If you believe this was a mistake, please email ASF User Services at: uso@asf.alaska.edu' ) elif application_status == APPLICATION_APPROVED: - raise CannotUpdateUserError(f'The application for user {user_id} has already been approved.') + raise ApplicationClosedError(f'The application for user {user_id} has already been approved.') else: raise ValueError(f'User {user_id} has an invalid application status: {application_status}') From 82c2ebcc8ad9173b1b7c40d637045aab233bb0a6 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 2 May 2024 10:35:52 -0800 Subject: [PATCH 22/84] add TODOs --- apps/api/src/hyp3_api/routes.py | 1 + lib/dynamo/dynamo/user.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/apps/api/src/hyp3_api/routes.py b/apps/api/src/hyp3_api/routes.py index 97f3d4c27..7a154e45b 100644 --- a/apps/api/src/hyp3_api/routes.py +++ b/apps/api/src/hyp3_api/routes.py @@ -151,6 +151,7 @@ def jobs_get_by_job_id(job_id): @app.route('/user', methods=['PATCH']) @openapi def user_patch(): + # TODO: return user fields handlers.patch_user(request.get_json(), g.user) return {} diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 5a58aafdb..1e21535b9 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -21,6 +21,8 @@ class ApplicationClosedError(Exception): """Raised when the user attempts to update an application that has already been approved or rejected.""" +# TODO: return the user record via the ReturnValues param: +# https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/dynamodb/table/update_item.html def update_user(user_id: str, body: dict) -> None: # TODO also set derived EDL fields user = get_or_create_user(user_id) From 19d5a356c3e7f7f67f02096c2b07e911b8948075 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 2 May 2024 11:35:36 -0800 Subject: [PATCH 23/84] set EDL email, name, country --- apps/api/src/hyp3_api/auth.py | 9 +++------ apps/api/src/hyp3_api/handlers.py | 4 ++-- apps/api/src/hyp3_api/routes.py | 9 +++++---- lib/dynamo/dynamo/user.py | 22 +++++++++++++++++++--- 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/apps/api/src/hyp3_api/auth.py b/apps/api/src/hyp3_api/auth.py index 6927798ec..128ac303d 100644 --- a/apps/api/src/hyp3_api/auth.py +++ b/apps/api/src/hyp3_api/auth.py @@ -1,16 +1,13 @@ import time from os import environ +from typing import Optional import jwt -def decode_token(token): +def decode_token(token) -> Optional[dict]: try: - payload = jwt.decode(token, environ['AUTH_PUBLIC_KEY'], algorithms=environ['AUTH_ALGORITHM']) - return { - 'active': True, - 'sub': payload['urs-user-id'], - } + return jwt.decode(token, environ['AUTH_PUBLIC_KEY'], algorithms=environ['AUTH_ALGORITHM']) except (jwt.ExpiredSignatureError, jwt.DecodeError): return None diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 9b2d367ed..9f53f73ff 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -69,10 +69,10 @@ def get_names_for_user(user): return sorted(list(names)) -def patch_user(body: dict, user: str) -> None: +def patch_user(body: dict, user: str, earthdata_info: dict) -> None: print(body) try: - dynamo.user.update_user(user, body) + dynamo.user.update_user(user, earthdata_info, body) except dynamo.user.ApplicationClosedError as e: # TODO is this the appropriate status code? abort(problem_format(403, str(e))) diff --git a/apps/api/src/hyp3_api/routes.py b/apps/api/src/hyp3_api/routes.py index 7a154e45b..9f317549d 100644 --- a/apps/api/src/hyp3_api/routes.py +++ b/apps/api/src/hyp3_api/routes.py @@ -39,9 +39,10 @@ def check_system_available(): @app.before_request def authenticate_user(): cookie = request.cookies.get('asf-urs') - auth_info = auth.decode_token(cookie) - if auth_info is not None: - g.user = auth_info['sub'] + payload = auth.decode_token(cookie) + if payload is not None: + g.user = payload['urs-user-id'] + g.earthdata_info = {key: payload[key] for key in ['first_name', 'last_name', 'urs-access-token']} else: if any([request.path.startswith(route) for route in AUTHENTICATED_ROUTES]) and request.method != 'OPTIONS': abort(handlers.problem_format(401, 'No authorization token provided')) @@ -152,7 +153,7 @@ def jobs_get_by_job_id(job_id): @openapi def user_patch(): # TODO: return user fields - handlers.patch_user(request.get_json(), g.user) + handlers.patch_user(request.get_json(), g.user, g.earthdata_info) return {} diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 1e21535b9..c1e50fcb9 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -4,6 +4,7 @@ from os import environ import botocore.exceptions +import requests from dynamo.util import DYNAMODB_RESOURCE @@ -23,8 +24,8 @@ class ApplicationClosedError(Exception): # TODO: return the user record via the ReturnValues param: # https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/dynamodb/table/update_item.html -def update_user(user_id: str, body: dict) -> None: - # TODO also set derived EDL fields +def update_user(user_id: str, earthdata_info: dict, body: dict) -> None: + email_address, country = _get_email_country(user_id, earthdata_info) user = get_or_create_user(user_id) application_status = user['application_status'] if application_status in (APPLICATION_NOT_STARTED, APPLICATION_PENDING): @@ -32,9 +33,16 @@ def update_user(user_id: str, body: dict) -> None: try: users_table.update_item( Key={'user_id': user_id}, - UpdateExpression='SET use_case = :use_case, application_status = :pending', + UpdateExpression=( + 'SET email_address = :email_address, first_name = :first_name, last_name = :last_name' + ' country = :country, use_case = :use_case, application_status = :pending' + ), ConditionExpression='application_status IN (:not_started, :pending)', ExpressionAttributeValues={ + ':email_address': email_address, + ':first_name': earthdata_info['first_name'], + ':last_name': earthdata_info['last_name'], + ':country': country, ':use_case': body['use_case'], ':not_started': APPLICATION_NOT_STARTED, ':pending': APPLICATION_PENDING @@ -55,6 +63,14 @@ def update_user(user_id: str, body: dict) -> None: raise ValueError(f'User {user_id} has an invalid application status: {application_status}') +def _get_email_country(user_id: str, earthdata_info: dict) -> tuple[str, str]: + url = f'https://urs.earthdata.nasa.gov/api/users/{user_id}' + response = requests.get(url, headers={'Authorization': f'Bearer {earthdata_info["urs-access-token"]}'}) + response.raise_for_status() + payload = response.json() + return payload['email_address'], payload['country'] + + def get_or_create_user(user_id: str) -> dict: current_month = _get_current_month() default_credits = Decimal(os.environ['DEFAULT_CREDITS_PER_USER']) From 49f7e3e2e1b5aca5624cc8f820c553e37ac0c0af Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 2 May 2024 13:11:15 -0800 Subject: [PATCH 24/84] missing comma --- 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 c1e50fcb9..ad6d24d65 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -34,7 +34,7 @@ def update_user(user_id: str, earthdata_info: dict, body: dict) -> None: users_table.update_item( Key={'user_id': user_id}, UpdateExpression=( - 'SET email_address = :email_address, first_name = :first_name, last_name = :last_name' + 'SET email_address = :email_address, first_name = :first_name, last_name = :last_name,' ' country = :country, use_case = :use_case, application_status = :pending' ), ConditionExpression='application_status IN (:not_started, :pending)', From 93a6c16baba7b9be9da0f4403cf39a8a3473bfd2 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 2 May 2024 13:48:23 -0800 Subject: [PATCH 25/84] return full user record from both /user endpoints --- .../src/hyp3_api/api-spec/openapi-spec.yml.j2 | 4 ++ apps/api/src/hyp3_api/handlers.py | 41 ++++++++++--------- apps/api/src/hyp3_api/routes.py | 4 +- lib/dynamo/dynamo/user.py | 30 +++++++------- 4 files changed, 40 insertions(+), 39 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 2ccf0c015..434017f0c 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 @@ -115,6 +115,10 @@ paths: responses: "200": description: 200 response + content: + application/json: + schema: + $ref: "#/components/schemas/user" get: description: Get information about the logged in user. diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 9f53f73ff..219efcea8 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -60,31 +60,32 @@ def get_job_by_id(job_id): return job -def get_names_for_user(user): - jobs, next_key = dynamo.jobs.query_jobs(user) - while next_key is not None: - new_jobs, next_key = dynamo.jobs.query_jobs(user, start_key=next_key) - jobs.extend(new_jobs) - names = {job['name'] for job in jobs if 'name' in job} - return sorted(list(names)) - - -def patch_user(body: dict, user: str, earthdata_info: dict) -> None: +def patch_user(body: dict, user: str, earthdata_info: dict) -> dict: print(body) try: - dynamo.user.update_user(user, earthdata_info, body) + user_record = dynamo.user.update_user(user, earthdata_info, body) except dynamo.user.ApplicationClosedError as e: - # TODO is this the appropriate status code? abort(problem_format(403, str(e))) + return _user_response(user_record) def get_user(user): user_record = dynamo.user.get_or_create_user(user) - return { - 'user_id': user, - 'application_status': user_record['application_status'], - 'use_case': user_record.get('use_case'), - '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), - } + return _user_response(user_record) + + +def _user_response(user_record: dict) -> dict: + # TODO: count this as jobs are submitted, not every time `/user` is queried + job_names = _get_names_for_user(user_record['user_id']) + payload = {key: user_record[key] for key in user_record if not key.startswith('_')} + payload['job_names'] = job_names + return payload + + +def _get_names_for_user(user): + jobs, next_key = dynamo.jobs.query_jobs(user) + while next_key is not None: + new_jobs, next_key = dynamo.jobs.query_jobs(user, start_key=next_key) + jobs.extend(new_jobs) + names = {job['name'] for job in jobs if 'name' in job} + return sorted(list(names)) diff --git a/apps/api/src/hyp3_api/routes.py b/apps/api/src/hyp3_api/routes.py index 9f317549d..17de9b60d 100644 --- a/apps/api/src/hyp3_api/routes.py +++ b/apps/api/src/hyp3_api/routes.py @@ -152,9 +152,7 @@ def jobs_get_by_job_id(job_id): @app.route('/user', methods=['PATCH']) @openapi def user_patch(): - # TODO: return user fields - handlers.patch_user(request.get_json(), g.user, g.earthdata_info) - return {} + return jsonify(handlers.patch_user(request.get_json(), g.user, g.earthdata_info)) @app.route('/user', methods=['GET']) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index ad6d24d65..8243d4b4e 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -22,16 +22,14 @@ class ApplicationClosedError(Exception): """Raised when the user attempts to update an application that has already been approved or rejected.""" -# TODO: return the user record via the ReturnValues param: -# https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/dynamodb/table/update_item.html -def update_user(user_id: str, earthdata_info: dict, body: dict) -> None: - email_address, country = _get_email_country(user_id, earthdata_info) +def update_user(user_id: str, earthdata_info: dict, body: dict) -> dict: user = get_or_create_user(user_id) application_status = user['application_status'] if application_status in (APPLICATION_NOT_STARTED, APPLICATION_PENDING): + email_address, country = _get_email_country(user_id, earthdata_info) users_table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) try: - users_table.update_item( + user = users_table.update_item( Key={'user_id': user_id}, UpdateExpression=( 'SET email_address = :email_address, first_name = :first_name, last_name = :last_name,' @@ -47,20 +45,21 @@ def update_user(user_id: str, earthdata_info: dict, body: dict) -> None: ':not_started': APPLICATION_NOT_STARTED, ':pending': APPLICATION_PENDING }, - ) + ReturnValues='ALL_NEW', + )['Attributes'] except botocore.exceptions.ClientError as e: if e.response['Error']['Code'] == 'ConditionalCheckFailedException': raise DatabaseConditionException(f'Failed to update record for user {user_id}') raise - elif application_status == APPLICATION_REJECTED: + return user + if application_status == APPLICATION_REJECTED: raise ApplicationClosedError( f'Unfortunately, the application for user {user_id} has been rejected.' ' If you believe this was a mistake, please email ASF User Services at: uso@asf.alaska.edu' ) - elif application_status == APPLICATION_APPROVED: + if application_status == APPLICATION_APPROVED: raise ApplicationClosedError(f'The application for user {user_id} has already been approved.') - else: - raise ValueError(f'User {user_id} has an invalid application status: {application_status}') + raise ValueError(f'User {user_id} has an invalid application status: {application_status}') def _get_email_country(user_id: str, earthdata_info: dict) -> tuple[str, str]: @@ -98,11 +97,10 @@ def _get_current_month() -> str: def _create_user(user_id: str, users_table) -> dict: - # TODO rename month_of_last_credits_reset -> month_of_last_credit_reset user = { 'user_id': user_id, 'remaining_credits': Decimal(0), - 'month_of_last_credits_reset': '0', + '_month_of_last_credit_reset': '0', 'application_status': APPLICATION_NOT_STARTED } try: @@ -118,17 +116,17 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month if ( os.environ['RESET_CREDITS_MONTHLY'] == 'true' and user['application_status'] == APPLICATION_APPROVED - and user['month_of_last_credits_reset'] < current_month # noqa: W503 + and user['_month_of_last_credit_reset'] < current_month # noqa: W503 and user['remaining_credits'] is not None # noqa: W503 ): - user['month_of_last_credits_reset'] = current_month + user['_month_of_last_credit_reset'] = current_month user['remaining_credits'] = user.get('credits_per_month', default_credits) try: users_table.put_item( Item=user, ConditionExpression=( 'application_status = :approved' - ' AND month_of_last_credits_reset < :current_month' + ' AND _month_of_last_credit_reset < :current_month' ' AND attribute_type(remaining_credits, :number)' ), ExpressionAttributeValues={ @@ -146,7 +144,7 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month elif user['application_status'] != APPLICATION_APPROVED: # TODO should we also check RESET_CREDITS_MONTHLY for this case? should we perhaps get rid of that # variable for now since hyp3-enterprise-test is the only deployment that sets it false? - user['month_of_last_credits_reset'] = '0' + user['_month_of_last_credit_reset'] = '0' user['remaining_credits'] = Decimal(0) try: users_table.put_item( From 917ffa88b95c9a406fc3bf827c8e1abb0f2a5393 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 2 May 2024 14:05:22 -0800 Subject: [PATCH 26/84] dynamodb is weird --- lib/dynamo/dynamo/user.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 8243d4b4e..9c0bad63e 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -126,9 +126,10 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month Item=user, ConditionExpression=( 'application_status = :approved' - ' AND _month_of_last_credit_reset < :current_month' + ' AND #month_of_last_credit_reset < :current_month' ' AND attribute_type(remaining_credits, :number)' ), + ExpressionAttributeNames={'#month_of_last_credit_reset': '_month_of_last_credit_reset'}, ExpressionAttributeValues={ ':approved': APPLICATION_APPROVED, ':current_month': current_month, From 14fd8cc367a52e2879c6fb071198b2cc4b38d3aa Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 2 May 2024 14:18:14 -0800 Subject: [PATCH 27/84] update user schema --- .../src/hyp3_api/api-spec/openapi-spec.yml.j2 | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 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 434017f0c..7536762eb 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 @@ -202,16 +202,32 @@ components: description: Information about a user type: object required: - - user_id + - application_status + - job_names - remaining_credits + - user_id additionalProperties: false properties: - user_id: - $ref: "#/components/schemas/user_id" - remaining_credits: + application_status: + type: string + country: + type: string + credits_per_month: $ref: "#/components/schemas/credits" + email_address: + type: string + first_name: + type: string job_names: $ref: "#/components/schemas/job_names_list" + last_name: + type: string + remaining_credits: + $ref: "#/components/schemas/credits" + use_case: + type: string + user_id: + $ref: "#/components/schemas/user_id" credits: description: Processing credits for running jobs. From 8a66945a62a6fa65589d64e37b8203a68d03b8ed Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 2 May 2024 14:27:32 -0800 Subject: [PATCH 28/84] remove the `reset_credits_monthly` deployment param --- .github/actions/deploy-hyp3/action.yml | 4 --- .github/workflows/deploy-daac.yml | 3 -- .github/workflows/deploy-enterprise-test.yml | 4 --- .github/workflows/deploy-enterprise.yml | 13 --------- .../workflows/deploy-whitelisting-sandbox.yml | 2 -- apps/api/api-cf.yml.j2 | 1 - lib/dynamo/dynamo/user.py | 5 +--- tests/cfg.env | 1 - tests/test_dynamo/test_user.py | 29 ------------------- 9 files changed, 1 insertion(+), 61 deletions(-) diff --git a/.github/actions/deploy-hyp3/action.yml b/.github/actions/deploy-hyp3/action.yml index 6d5fa269d..9806764a4 100644 --- a/.github/actions/deploy-hyp3/action.yml +++ b/.github/actions/deploy-hyp3/action.yml @@ -38,9 +38,6 @@ inputs: DEFAULT_CREDITS_PER_USER: description: "The default number of credits given to a new user" required: true - RESET_CREDITS_MONTHLY: - description: "Whether to reset each user's remaining credits each month" - required: true COST_PROFILE: description: "Job spec cost profile" required: true @@ -126,7 +123,6 @@ 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-daac.yml b/.github/workflows/deploy-daac.yml index ee30d6f56..667cea63f 100644 --- a/.github/workflows/deploy-daac.yml +++ b/.github/workflows/deploy-daac.yml @@ -22,7 +22,6 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 10000 - reset_credits_monthly: true cost_profile: EDC 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 @@ -41,7 +40,6 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 10000 - reset_credits_monthly: true cost_profile: EDC deploy_ref: refs/heads/develop job_files: >- @@ -90,7 +88,6 @@ 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 }} COST_PROFILE: ${{ matrix.cost_profile }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} diff --git a/.github/workflows/deploy-enterprise-test.yml b/.github/workflows/deploy-enterprise-test.yml index ec505a65c..a5731d068 100644 --- a/.github/workflows/deploy-enterprise-test.yml +++ b/.github/workflows/deploy-enterprise-test.yml @@ -20,7 +20,6 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: false cost_profile: DEFAULT deploy_ref: refs/heads/develop job_files: >- @@ -44,7 +43,6 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: true cost_profile: DEFAULT job_files: >- job_spec/ARIA_RAIDER.yml @@ -63,7 +61,6 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: true cost_profile: DEFAULT job_files: >- job_spec/AUTORIFT_ITS_LIVE.yml @@ -109,7 +106,6 @@ 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 }} COST_PROFILE: ${{ matrix.cost_profile }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} diff --git a/.github/workflows/deploy-enterprise.yml b/.github/workflows/deploy-enterprise.yml index b3ef70364..04405e3f3 100644 --- a/.github/workflows/deploy-enterprise.yml +++ b/.github/workflows/deploy-enterprise.yml @@ -20,7 +20,6 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: true cost_profile: DEFAULT job_files: >- job_spec/AUTORIFT_ITS_LIVE.yml @@ -39,7 +38,6 @@ jobs: image_tag: latest product_lifetime_in_days: 180 default_credits_per_user: 0 - reset_credits_monthly: true cost_profile: DEFAULT job_files: >- job_spec/ARIA_RAIDER.yml @@ -58,7 +56,6 @@ jobs: image_tag: latest product_lifetime_in_days: 30 default_credits_per_user: 0 - reset_credits_monthly: true cost_profile: DEFAULT job_files: job_spec/INSAR_ISCE.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge @@ -75,7 +72,6 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: true cost_profile: DEFAULT job_files: job_spec/INSAR_ISCE.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge @@ -92,7 +88,6 @@ jobs: image_tag: latest product_lifetime_in_days: 365000 default_credits_per_user: 0 - reset_credits_monthly: true cost_profile: DEFAULT 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 @@ -109,7 +104,6 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: true cost_profile: DEFAULT 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 @@ -126,7 +120,6 @@ jobs: image_tag: latest product_lifetime_in_days: 90 default_credits_per_user: 0 - reset_credits_monthly: true cost_profile: DEFAULT 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 @@ -143,7 +136,6 @@ jobs: image_tag: latest product_lifetime_in_days: 30 default_credits_per_user: 0 - reset_credits_monthly: true cost_profile: DEFAULT job_files: job_spec/INSAR_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 @@ -160,7 +152,6 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: true cost_profile: DEFAULT 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 @@ -177,7 +168,6 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 - reset_credits_monthly: true cost_profile: DEFAULT 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 @@ -194,7 +184,6 @@ jobs: image_tag: latest product_lifetime_in_days: 30 default_credits_per_user: 0 - reset_credits_monthly: true cost_profile: DEFAULT 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 @@ -213,7 +202,6 @@ 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: true cost_profile: DEFAULT 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 @@ -256,7 +244,6 @@ 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 }} COST_PROFILE: ${{ matrix.cost_profile }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} diff --git a/.github/workflows/deploy-whitelisting-sandbox.yml b/.github/workflows/deploy-whitelisting-sandbox.yml index 7f01b5358..d79478a53 100644 --- a/.github/workflows/deploy-whitelisting-sandbox.yml +++ b/.github/workflows/deploy-whitelisting-sandbox.yml @@ -20,7 +20,6 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 10 - reset_credits_monthly: true cost_profile: EDC deploy_ref: refs/heads/whitelisting-sandbox job_files: >- @@ -68,7 +67,6 @@ 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 }} COST_PROFILE: ${{ matrix.cost_profile }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} diff --git a/apps/api/api-cf.yml.j2 b/apps/api/api-cf.yml.j2 index 94f28cc8f..5243527fc 100644 --- a/apps/api/api-cf.yml.j2 +++ b/apps/api/api-cf.yml.j2 @@ -182,7 +182,6 @@ 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/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 9c0bad63e..ce13eeb14 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -114,8 +114,7 @@ def _create_user(user_id: str, users_table) -> dict: def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> dict: if ( - os.environ['RESET_CREDITS_MONTHLY'] == 'true' - and user['application_status'] == APPLICATION_APPROVED + user['application_status'] == APPLICATION_APPROVED and user['_month_of_last_credit_reset'] < current_month # noqa: W503 and user['remaining_credits'] is not None # noqa: W503 ): @@ -143,8 +142,6 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month ) raise elif user['application_status'] != APPLICATION_APPROVED: - # TODO should we also check RESET_CREDITS_MONTHLY for this case? should we perhaps get rid of that - # variable for now since hyp3-enterprise-test is the only deployment that sets it false? user['_month_of_last_credit_reset'] = '0' user['remaining_credits'] = Decimal(0) try: diff --git a/tests/cfg.env b/tests/cfg.env index 42d4b557b..a01fd7e08 100644 --- a/tests/cfg.env +++ b/tests/cfg.env @@ -4,7 +4,6 @@ USERS_TABLE_NAME=hyp3-db-table-user AUTH_PUBLIC_KEY=123456789 AUTH_ALGORITHM=HS256 DEFAULT_CREDITS_PER_USER=25 -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 0dbab7679..e487911af 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -77,8 +77,6 @@ def test_create_user_already_exists(tables): def test_reset_credits(tables, monkeypatch): - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'true') - original_user_record = { 'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01' } @@ -96,8 +94,6 @@ def test_reset_credits(tables, monkeypatch): def test_reset_credits_override(tables, monkeypatch): - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'true') - original_user_record = { 'user_id': 'foo', 'remaining_credits': Decimal(10), @@ -122,28 +118,7 @@ def test_reset_credits_override(tables, monkeypatch): assert tables.users_table.scan()['Items'] == [user] -def test_reset_credits_no_reset(tables, monkeypatch): - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'false') - - 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 tables.users_table.scan()['Items'] == [user] - - def test_reset_credits_same_month(tables, monkeypatch): - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'true') - original_user_record = { 'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-02' } @@ -161,8 +136,6 @@ def test_reset_credits_same_month(tables, monkeypatch): def test_reset_credits_infinite_credits(tables, monkeypatch): - monkeypatch.setenv('RESET_CREDITS_MONTHLY', 'true') - original_user_record = { 'user_id': 'foo', 'remaining_credits': None, 'month_of_last_credits_reset': '2024-01' } @@ -180,7 +153,6 @@ def test_reset_credits_infinite_credits(tables, monkeypatch): def test_reset_credits_failed_same_month(tables, monkeypatch): - 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 +171,6 @@ def test_reset_credits_failed_same_month(tables, monkeypatch): def test_reset_credits_failed_infinite_credits(tables, monkeypatch): - 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 4e952826d85a436cea6b5a13d511aca98f3216c7 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 2 May 2024 14:32:06 -0800 Subject: [PATCH 29/84] remove more reset credits monthly occurrences --- apps/api/api-cf.yml.j2 | 3 --- apps/main-cf.yml.j2 | 8 -------- docs/deployments/ASF-deployment.md | 3 +-- docs/deployments/JPL-deployment.md | 3 +-- 4 files changed, 2 insertions(+), 15 deletions(-) diff --git a/apps/api/api-cf.yml.j2 b/apps/api/api-cf.yml.j2 index 5243527fc..2edf56f22 100644 --- a/apps/api/api-cf.yml.j2 +++ b/apps/api/api-cf.yml.j2 @@ -15,9 +15,6 @@ Parameters: DefaultCreditsPerUser: Type: Number - ResetCreditsMonthly: - Type: String - SystemAvailable: Type: String diff --git a/apps/main-cf.yml.j2 b/apps/main-cf.yml.j2 index df7d00fd5..b0fda59b8 100644 --- a/apps/main-cf.yml.j2 +++ b/apps/main-cf.yml.j2 @@ -36,13 +36,6 @@ Parameters: Type: Number MinValue: 0 - ResetCreditsMonthly: - Description: "Whether to reset each user's remaining credits each month" - Type: String - AllowedValues: - - false - - true - SystemAvailable: Description: Set to false to shutdown system, API will run and provide errors to users, but will not accept jobs. Type: String @@ -123,7 +116,6 @@ 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 11c74e6e4..38c1a1489 100644 --- a/docs/deployments/ASF-deployment.md +++ b/docs/deployments/ASF-deployment.md @@ -93,6 +93,5 @@ aws cloudformation deploy \ DomainName= \ CertificateArn= \ SecretArn= \ - DefaultCreditsPerUser=0 \ - ResetCreditsMonthly=true + DefaultCreditsPerUser=0 ``` diff --git a/docs/deployments/JPL-deployment.md b/docs/deployments/JPL-deployment.md index eca192a3b..45b9d25e2 100644 --- a/docs/deployments/JPL-deployment.md +++ b/docs/deployments/JPL-deployment.md @@ -93,8 +93,7 @@ aws cloudformation deploy \ DomainName= \ CertificateArn= \ SecretArn= \ - DefaultCreditsPerUser=0 \ - ResetCreditsMonthly=true + DefaultCreditsPerUser=0 ``` ## 5. Post deployment From 461bd4ed1e1884a5fad5f8856180e04f90acb1f3 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 3 May 2024 09:39:01 -0800 Subject: [PATCH 30/84] finish API spec changes for whitelisting --- .../src/hyp3_api/api-spec/openapi-spec.yml.j2 | 50 ++++++++++++++++--- 1 file changed, 43 insertions(+), 7 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 7536762eb..1cb351da9 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 @@ -196,7 +196,7 @@ components: additionalProperties: false properties: use_case: - type: string + $ref: "#/components/schemas/use_case" user: description: Information about a user @@ -209,36 +209,66 @@ components: additionalProperties: false properties: application_status: - type: string + $ref: "#/components/schemas/application_status" country: - type: string + $ref: "#/components/schemas/country" credits_per_month: $ref: "#/components/schemas/credits" email_address: - type: string + $ref: "#/components/schemas/email_address" first_name: - type: string + $ref: "#/components/schemas/first_name" job_names: $ref: "#/components/schemas/job_names_list" last_name: - type: string + $ref: "#/components/schemas/last_name" remaining_credits: $ref: "#/components/schemas/credits" use_case: - type: string + $ref: "#/components/schemas/use_case" user_id: $ref: "#/components/schemas/user_id" + application_status: + description: Status of an application for processing approval. + type: string + enum: + - NOT STARTED + - PENDING + - APPROVED + - REJECTED + example: PENDING + + country: + description: Earthdata Login country. + type: string + example: United States + credits: description: Processing credits for running jobs. type: number minimum: 0 + email_address: + description: Earthdata Login email address. + type: string + example: user@example.com + + first_name: + description: Earthdata Login first name. + type: string + example: First + job_names_list: type: array items: $ref: "#/components/schemas/name" + last_name: + description: Earthdata Login last name. + type: string + example: Last + list_of_new_jobs: description: Contains a list of new job objects. type: array @@ -308,6 +338,12 @@ components: format: uuid example: 27836b79-e5b2-4d8f-932f-659724ea02c3 + use_case: + description: Reason for wanting to use HyP3. + type: string + # TODO maybe we should come up with a good example here + example: I want to process data. + user_id: description: Username from Earthdata Login. type: string From 91252bae2d156719e99360f9a1d15134dc02e115 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 3 May 2024 11:27:40 -0800 Subject: [PATCH 31/84] set full edl profile in user record --- apps/api/src/hyp3_api/handlers.py | 4 ++-- apps/api/src/hyp3_api/routes.py | 4 ++-- lib/dynamo/dynamo/user.py | 22 ++++++++-------------- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 219efcea8..791731892 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -60,10 +60,10 @@ def get_job_by_id(job_id): return job -def patch_user(body: dict, user: str, earthdata_info: dict) -> dict: +def patch_user(body: dict, user: str, urs_access_token: str) -> dict: print(body) try: - user_record = dynamo.user.update_user(user, earthdata_info, body) + user_record = dynamo.user.update_user(user, urs_access_token, body) except dynamo.user.ApplicationClosedError as e: abort(problem_format(403, str(e))) return _user_response(user_record) diff --git a/apps/api/src/hyp3_api/routes.py b/apps/api/src/hyp3_api/routes.py index 17de9b60d..e96b1daa2 100644 --- a/apps/api/src/hyp3_api/routes.py +++ b/apps/api/src/hyp3_api/routes.py @@ -42,7 +42,7 @@ def authenticate_user(): payload = auth.decode_token(cookie) if payload is not None: g.user = payload['urs-user-id'] - g.earthdata_info = {key: payload[key] for key in ['first_name', 'last_name', 'urs-access-token']} + g.urs_access_token = payload['urs-access-token'] else: if any([request.path.startswith(route) for route in AUTHENTICATED_ROUTES]) and request.method != 'OPTIONS': abort(handlers.problem_format(401, 'No authorization token provided')) @@ -152,7 +152,7 @@ def jobs_get_by_job_id(job_id): @app.route('/user', methods=['PATCH']) @openapi def user_patch(): - return jsonify(handlers.patch_user(request.get_json(), g.user, g.earthdata_info)) + return jsonify(handlers.patch_user(request.get_json(), g.user, g.urs_access_token)) @app.route('/user', methods=['GET']) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index ce13eeb14..902402c55 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -22,25 +22,20 @@ class ApplicationClosedError(Exception): """Raised when the user attempts to update an application that has already been approved or rejected.""" -def update_user(user_id: str, earthdata_info: dict, body: dict) -> dict: +def update_user(user_id: str, urs_access_token: str, body: dict) -> dict: user = get_or_create_user(user_id) application_status = user['application_status'] if application_status in (APPLICATION_NOT_STARTED, APPLICATION_PENDING): - email_address, country = _get_email_country(user_id, earthdata_info) + edl_profile = _get_edl_profile(user_id, urs_access_token) users_table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) try: user = users_table.update_item( Key={'user_id': user_id}, - UpdateExpression=( - 'SET email_address = :email_address, first_name = :first_name, last_name = :last_name,' - ' country = :country, use_case = :use_case, application_status = :pending' - ), + UpdateExpression='SET #edl_profile = :edl_profile, use_case = :use_case, application_status = :pending', ConditionExpression='application_status IN (:not_started, :pending)', + ExpressionAttributeNames={'#edl_profile': '_edl_profile'}, ExpressionAttributeValues={ - ':email_address': email_address, - ':first_name': earthdata_info['first_name'], - ':last_name': earthdata_info['last_name'], - ':country': country, + ':edl_profile': edl_profile, ':use_case': body['use_case'], ':not_started': APPLICATION_NOT_STARTED, ':pending': APPLICATION_PENDING @@ -62,12 +57,11 @@ def update_user(user_id: str, earthdata_info: dict, body: dict) -> dict: raise ValueError(f'User {user_id} has an invalid application status: {application_status}') -def _get_email_country(user_id: str, earthdata_info: dict) -> tuple[str, str]: +def _get_edl_profile(user_id: str, urs_access_token: str) -> dict: url = f'https://urs.earthdata.nasa.gov/api/users/{user_id}' - response = requests.get(url, headers={'Authorization': f'Bearer {earthdata_info["urs-access-token"]}'}) + response = requests.get(url, headers={'Authorization': f'Bearer {urs_access_token}'}) response.raise_for_status() - payload = response.json() - return payload['email_address'], payload['country'] + return response.json() def get_or_create_user(user_id: str) -> dict: From b26fddf5bf0b9334bc90a6ed9625d72f6cc691ab Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 3 May 2024 11:32:18 -0800 Subject: [PATCH 32/84] urs_access_token -> edl_access_token --- apps/api/src/hyp3_api/handlers.py | 4 ++-- apps/api/src/hyp3_api/routes.py | 4 ++-- lib/dynamo/dynamo/user.py | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 791731892..06d3b31ad 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -60,10 +60,10 @@ def get_job_by_id(job_id): return job -def patch_user(body: dict, user: str, urs_access_token: str) -> dict: +def patch_user(body: dict, user: str, edl_access_token: str) -> dict: print(body) try: - user_record = dynamo.user.update_user(user, urs_access_token, body) + user_record = dynamo.user.update_user(user, edl_access_token, body) except dynamo.user.ApplicationClosedError as e: abort(problem_format(403, str(e))) return _user_response(user_record) diff --git a/apps/api/src/hyp3_api/routes.py b/apps/api/src/hyp3_api/routes.py index e96b1daa2..e3c820d05 100644 --- a/apps/api/src/hyp3_api/routes.py +++ b/apps/api/src/hyp3_api/routes.py @@ -42,7 +42,7 @@ def authenticate_user(): payload = auth.decode_token(cookie) if payload is not None: g.user = payload['urs-user-id'] - g.urs_access_token = payload['urs-access-token'] + g.edl_access_token = payload['urs-access-token'] else: if any([request.path.startswith(route) for route in AUTHENTICATED_ROUTES]) and request.method != 'OPTIONS': abort(handlers.problem_format(401, 'No authorization token provided')) @@ -152,7 +152,7 @@ def jobs_get_by_job_id(job_id): @app.route('/user', methods=['PATCH']) @openapi def user_patch(): - return jsonify(handlers.patch_user(request.get_json(), g.user, g.urs_access_token)) + return jsonify(handlers.patch_user(request.get_json(), g.user, g.edl_access_token)) @app.route('/user', methods=['GET']) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 902402c55..605cddd74 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -22,11 +22,11 @@ class ApplicationClosedError(Exception): """Raised when the user attempts to update an application that has already been approved or rejected.""" -def update_user(user_id: str, urs_access_token: str, body: dict) -> dict: +def update_user(user_id: str, edl_access_token: str, body: dict) -> dict: user = get_or_create_user(user_id) application_status = user['application_status'] if application_status in (APPLICATION_NOT_STARTED, APPLICATION_PENDING): - edl_profile = _get_edl_profile(user_id, urs_access_token) + edl_profile = _get_edl_profile(user_id, edl_access_token) users_table = DYNAMODB_RESOURCE.Table(environ['USERS_TABLE_NAME']) try: user = users_table.update_item( @@ -57,9 +57,9 @@ def update_user(user_id: str, urs_access_token: str, body: dict) -> dict: raise ValueError(f'User {user_id} has an invalid application status: {application_status}') -def _get_edl_profile(user_id: str, urs_access_token: str) -> dict: +def _get_edl_profile(user_id: str, edl_access_token: str) -> dict: url = f'https://urs.earthdata.nasa.gov/api/users/{user_id}' - response = requests.get(url, headers={'Authorization': f'Bearer {urs_access_token}'}) + response = requests.get(url, headers={'Authorization': f'Bearer {edl_access_token}'}) response.raise_for_status() return response.json() From 86933a5a68b3d4e0cecbf557ea8aaea907e98a01 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 3 May 2024 11:42:43 -0800 Subject: [PATCH 33/84] add urs-access-token to test cookie payload --- apps/api/src/hyp3_api/auth.py | 3 ++- tests/test_api/conftest.py | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/api/src/hyp3_api/auth.py b/apps/api/src/hyp3_api/auth.py index 128ac303d..ff75ae6ff 100644 --- a/apps/api/src/hyp3_api/auth.py +++ b/apps/api/src/hyp3_api/auth.py @@ -12,9 +12,10 @@ def decode_token(token) -> Optional[dict]: return None -def get_mock_jwt_cookie(user: str, lifetime_in_seconds: int): +def get_mock_jwt_cookie(user: str, lifetime_in_seconds: int, access_token: str) -> str: payload = { 'urs-user-id': user, + 'urs-access-token': access_token, 'exp': int(time.time()) + lifetime_in_seconds, } value = jwt.encode( diff --git a/tests/test_api/conftest.py b/tests/test_api/conftest.py index 202a57a26..a70e0b332 100644 --- a/tests/test_api/conftest.py +++ b/tests/test_api/conftest.py @@ -14,6 +14,7 @@ DEFAULT_JOB_ID = 'myJobId' DEFAULT_USERNAME = 'test_username' +DEFAULT_ACCESS_TOKEN = 'test_access_token' CMR_URL_RE = re.compile(f'{CMR_URL}.*') @@ -24,11 +25,11 @@ def client(): yield test_client -def login(client, username=DEFAULT_USERNAME): +def login(client, username=DEFAULT_USERNAME, access_token=DEFAULT_ACCESS_TOKEN): client.set_cookie( domain='localhost', key=AUTH_COOKIE, - value=auth.get_mock_jwt_cookie(username, lifetime_in_seconds=10_000) + value=auth.get_mock_jwt_cookie(username, lifetime_in_seconds=10_000, access_token=access_token) ) From 293eb9f9d152970b163eb72df603f30c49156c85 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 3 May 2024 11:43:04 -0800 Subject: [PATCH 34/84] only reset credits for unapproved users if needed --- lib/dynamo/dynamo/user.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 605cddd74..f84b43219 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -135,14 +135,14 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month f'Failed to perform monthly credit reset for approved user {user["user_id"]}' ) raise - elif user['application_status'] != APPLICATION_APPROVED: + elif user['application_status'] != APPLICATION_APPROVED and user['remaining_credits'] != Decimal(0): user['_month_of_last_credit_reset'] = '0' user['remaining_credits'] = Decimal(0) try: users_table.put_item( Item=user, - ConditionExpression='NOT (application_status = :approved)', - ExpressionAttributeValues={':approved': APPLICATION_APPROVED}, + ConditionExpression='NOT (application_status = :approved) AND NOT (remaining_credits = :zero)', + ExpressionAttributeValues={':approved': APPLICATION_APPROVED, ':zero': Decimal(0)}, ) except botocore.exceptions.ClientError as e: if e.response['Error']['Code'] == 'ConditionalCheckFailedException': From 8820ff772c1dba314f7f91416a5f23b0359e4e74 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 3 May 2024 11:50:12 -0800 Subject: [PATCH 35/84] delete edl fields from api schema --- .../src/hyp3_api/api-spec/openapi-spec.yml.j2 | 28 ------------------- 1 file changed, 28 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 1cb351da9..f583d5287 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 @@ -210,18 +210,10 @@ components: properties: application_status: $ref: "#/components/schemas/application_status" - country: - $ref: "#/components/schemas/country" credits_per_month: $ref: "#/components/schemas/credits" - email_address: - $ref: "#/components/schemas/email_address" - first_name: - $ref: "#/components/schemas/first_name" job_names: $ref: "#/components/schemas/job_names_list" - last_name: - $ref: "#/components/schemas/last_name" remaining_credits: $ref: "#/components/schemas/credits" use_case: @@ -239,36 +231,16 @@ components: - REJECTED example: PENDING - country: - description: Earthdata Login country. - type: string - example: United States - credits: description: Processing credits for running jobs. type: number minimum: 0 - email_address: - description: Earthdata Login email address. - type: string - example: user@example.com - - first_name: - description: Earthdata Login first name. - type: string - example: First - job_names_list: type: array items: $ref: "#/components/schemas/name" - last_name: - description: Earthdata Login last name. - type: string - example: Last - list_of_new_jobs: description: Contains a list of new job objects. type: array From beacb000d0e6c7888a3452eb368b5331a56e91ff Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 3 May 2024 12:16:11 -0800 Subject: [PATCH 36/84] update api spec tests --- tests/test_api/test_api_spec.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_api/test_api_spec.py b/tests/test_api/test_api_spec.py index 027b7e60e..b9937f28a 100644 --- a/tests/test_api/test_api_spec.py +++ b/tests/test_api/test_api_spec.py @@ -7,7 +7,7 @@ ENDPOINTS = { JOBS_URI: {'GET', 'HEAD', 'OPTIONS', 'POST'}, JOBS_URI + '/foo': {'GET', 'HEAD', 'OPTIONS'}, - USER_URI: {'GET', 'HEAD', 'OPTIONS'}, + USER_URI: {'GET', 'HEAD', 'OPTIONS', 'PATCH'}, } @@ -52,7 +52,7 @@ def test_expired_cookie(client): client.set_cookie( domain='localhost', key=AUTH_COOKIE, - value=auth.get_mock_jwt_cookie('user', lifetime_in_seconds=-1) + value=auth.get_mock_jwt_cookie('user', lifetime_in_seconds=-1, access_token='token') ) response = client.get(uri) assert response.status_code == HTTPStatus.UNAUTHORIZED From 68b30158f08498507022abe132857f6511f9942f Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 3 May 2024 12:28:13 -0800 Subject: [PATCH 37/84] update api get user tests --- tests/test_api/test_get_user.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tests/test_api/test_get_user.py b/tests/test_api/test_get_user.py index af54b05f1..df86b3261 100644 --- a/tests/test_api/test_get_user.py +++ b/tests/test_api/test_get_user.py @@ -7,20 +7,19 @@ def test_get_new_user(client, tables, monkeypatch): - monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') - login(client, 'user') response = client.get(USER_URI) assert response.status_code == HTTPStatus.OK assert response.json == { 'user_id': 'user', - 'remaining_credits': 25, + 'application_status': 'NOT STARTED', + 'remaining_credits': 0, 'job_names': [], } -def test_get_existing_user(client, tables): - user = {'user_id': 'user', 'remaining_credits': None} +def test_get_rejected_user(client, tables): + user = {'user_id': 'user', 'remaining_credits': 100, 'application_status': 'REJECTED'} tables.users_table.put_item(Item=user) login(client, 'user') @@ -28,14 +27,22 @@ def test_get_existing_user(client, tables): assert response.status_code == HTTPStatus.OK assert response.json == { 'user_id': 'user', - 'remaining_credits': None, + 'application_status': 'REJECTED', + 'remaining_credits': 0, 'job_names': [], } def test_get_user_with_jobs(client, tables): user_id = 'user_with_jobs' - user = {'user_id': user_id, 'remaining_credits': 20, 'foo': 'bar'} + user = { + 'user_id': user_id, + 'remaining_credits': 20, + 'application_status': 'APPROVED', + 'credits_per_month': 50, + '_month_of_last_credit_reset': '2024-01-01', + '_foo': 'bar', + } tables.users_table.put_item(Item=user) request_time = format_time(datetime.now(timezone.utc)) @@ -53,7 +60,9 @@ def test_get_user_with_jobs(client, tables): assert response.status_code == HTTPStatus.OK assert response.json == { 'user_id': 'user_with_jobs', - 'remaining_credits': 20, + 'application_status': 'APPROVED', + 'credits_per_month': 50, + 'remaining_credits': 50, 'job_names': [ 'job1', 'job2', From 7d11134e403e5efe1ed1199cd144e65e641dc1b0 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 3 May 2024 13:01:12 -0800 Subject: [PATCH 38/84] update api submit job tests --- tests/conftest.py | 12 ++++++++ tests/test_api/test_submit_job.py | 50 +++++++++++++++---------------- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 3ba79fe42..574174351 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -41,5 +41,17 @@ class Tables: yield tables +@pytest.fixture +def approved_user(tables): + user = { + 'user_id': 'approved_user', + 'remaining_credits': 10000, + 'application_status': 'APPROVED', + '_month_of_last_credit_reset': '2024-01-01', + } + tables.users_table.put_item(Item=user) + return user + + def list_have_same_elements(l1, l2): return [item for item in l1 if item not in l2] == [] == [item for item in l2 if item not in l1] diff --git a/tests/test_api/test_submit_job.py b/tests/test_api/test_submit_job.py index 1a0a80f05..cc392f140 100644 --- a/tests/test_api/test_submit_job.py +++ b/tests/test_api/test_submit_job.py @@ -6,8 +6,8 @@ from dynamo.util import format_time -def test_submit_one_job(client, tables): - login(client) +def test_submit_one_job(client, approved_user): + login(client, username=approved_user['user_id']) batch = [make_job()] setup_requests_mock(batch) response = submit_batch(client, batch) @@ -16,11 +16,11 @@ def test_submit_one_job(client, tables): assert len(jobs) == 1 assert jobs[0]['status_code'] == 'PENDING' assert jobs[0]['request_time'] <= format_time(datetime.now(timezone.utc)) - assert jobs[0]['user_id'] == DEFAULT_USERNAME + assert jobs[0]['user_id'] == approved_user['user_id'] -def test_submit_insar_gamma(client, tables): - login(client) +def test_submit_insar_gamma(client, approved_user): + login(client, username=approved_user['user_id']) granules = [ 'S1A_IW_SLC__1SDV_20200720T172109_20200720T172128_033541_03E2FB_341F', 'S1A_IW_SLC__1SDV_20200813T172110_20200813T172129_033891_03EE3F_2C3E', @@ -55,8 +55,8 @@ def test_submit_insar_gamma(client, tables): assert response.status_code == HTTPStatus.OK -def test_submit_autorift(client, tables): - login(client) +def test_submit_autorift(client, approved_user): + login(client, username=approved_user['user_id']) job = make_job( [ 'S1A_IW_SLC__1SDV_20200720T172109_20200720T172128_033541_03E2FB_341F', @@ -70,8 +70,8 @@ def test_submit_autorift(client, tables): assert response.status_code == HTTPStatus.OK -def test_submit_multiple_job_types(client, tables): - login(client) +def test_submit_multiple_job_types(client, approved_user): + login(client, username=approved_user['user_id']) rtc_gamma_job = make_job() insar_gamma_job = make_job( [ @@ -93,9 +93,9 @@ def test_submit_multiple_job_types(client, tables): assert response.status_code == HTTPStatus.OK -def test_submit_many_jobs(client, tables): +def test_submit_many_jobs(client, approved_user): max_jobs = 25 - login(client) + login(client, username=approved_user['user_id']) batch = [make_job() for ii in range(max_jobs)] setup_requests_mock(batch) @@ -112,8 +112,8 @@ def test_submit_many_jobs(client, tables): assert response.status_code == HTTPStatus.BAD_REQUEST -def test_submit_exceeds_remaining_credits(client, tables, monkeypatch): - login(client) +def test_submit_exceeds_remaining_credits(client, approved_user, monkeypatch): + login(client, username=approved_user['user_id']) monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') batch1 = [make_job() for _ in range(20)] @@ -137,8 +137,8 @@ def test_submit_without_jobs(client): assert response.status_code == HTTPStatus.BAD_REQUEST -def test_submit_job_without_name(client, tables): - login(client) +def test_submit_job_without_name(client, approved_user): + login(client, username=approved_user['user_id']) batch = [ make_job(name=None) ] @@ -198,8 +198,8 @@ def test_submit_job_granule_does_not_exist(client, tables): 'S1A_IW_SLC__1SDV_20200610T173646_20200610T173704_032958_03D14C_5F2A' -def test_submit_good_rtc_granule_names(client, tables): - login(client) +def test_submit_good_rtc_granule_names(client, approved_user): + login(client, username=approved_user['user_id']) good_granule_names = [ 'S1B_IW_SLC__1SDV_20200604T082207_20200604T082234_021881_029874_5E38', 'S1A_IW_SLC__1SSH_20150608T205059_20150608T205126_006287_0083E8_C4F0', @@ -252,8 +252,8 @@ def test_submit_bad_rtc_granule_names(client): assert response.status_code == HTTPStatus.BAD_REQUEST -def test_submit_good_autorift_granule_names(client, tables): - login(client) +def test_submit_good_autorift_granule_names(client, approved_user): + login(client, username=approved_user['user_id']) good_granule_names = [ 'S1B_IW_SLC__1SDV_20200604T082207_20200604T082234_021881_029874_5E38', 'S1A_IW_SLC__1SSH_20150608T205059_20150608T205126_006287_0083E8_C4F0', @@ -318,8 +318,8 @@ def test_submit_bad_autorift_granule_names(client): assert response.status_code == HTTPStatus.BAD_REQUEST -def test_submit_mixed_job_parameters(client, tables): - login(client) +def test_submit_mixed_job_parameters(client, approved_user): + login(client, username=approved_user['user_id']) rtc_parameters = { 'resolution': 30.0, @@ -375,8 +375,8 @@ def test_submit_mixed_job_parameters(client, tables): assert response.status_code == HTTPStatus.BAD_REQUEST -def test_float_input(client, tables): - login(client) +def test_float_input(client, approved_user): + login(client, username=approved_user['user_id']) batch = [make_job(parameters={'resolution': 30.0})] setup_requests_mock(batch) response = submit_batch(client, batch) @@ -390,8 +390,8 @@ def test_float_input(client, tables): assert isinstance(response.json['jobs'][0]['job_parameters']['resolution'], int) -def test_submit_validate_only(client, tables): - login(client) +def test_submit_validate_only(client, tables, approved_user): + login(client, username=approved_user['user_id']) batch = [make_job()] setup_requests_mock(batch) From cdc9bbbb7471fb9e808ce11e7eb3e529743aa975 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 3 May 2024 14:30:23 -0800 Subject: [PATCH 39/84] fix some tests --- tests/test_dynamo/test_jobs.py | 50 ++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index b19ea7ed0..37de40eae 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -257,8 +257,15 @@ def test_get_credit_cost_validate_keys(): dynamo.jobs._get_credit_cost({'job_type': 'JOB_TYPE_F'}, costs) -def test_put_jobs(tables, monkeypatch): - monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '10') +def test_put_jobs(tables): + tables.users_table.put_item( + Item={ + 'user_id': 'user1', + 'remaining_credits': Decimal(10), + '_month_of_last_credit_reset': '2024-02', + 'application_status': 'APPROVED', + } + ) payload = [{'name': 'name1'}, {'name': 'name1'}, {'name': 'name2'}] with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month: @@ -281,12 +288,23 @@ def test_put_jobs(tables, monkeypatch): assert tables.jobs_table.scan()['Items'] == sorted(jobs, key=lambda item: item['job_id']) - assert tables.users_table.scan()['Items'] == [ - {'user_id': 'user1', 'remaining_credits': 7, 'month_of_last_credits_reset': '2024-02'} - ] + assert tables.users_table.scan()['Items'] == [{ + 'user_id': 'user1', + 'remaining_credits': 7, + '_month_of_last_credit_reset': '2024-02', + 'application_status': 'APPROVED', + }] def test_put_jobs_default_params(tables): + tables.users_table.put_item( + Item={ + 'user_id': 'user1', + 'remaining_credits': Decimal(0), + '_month_of_last_credit_reset': '0', + 'application_status': 'APPROVED', + } + ) default_params = { 'JOB_TYPE_A': {'a1': 'a1_default', 'a2': 'a2_default'}, 'JOB_TYPE_B': {'b1': 'b1_default'}, @@ -332,7 +350,15 @@ def test_put_jobs_default_params(tables): def test_put_jobs_costs(tables): - tables.users_table.put_item(Item={'user_id': 'user1', 'remaining_credits': Decimal(100)}) + tables.users_table.put_item( + Item={ + 'user_id': 'user1', + 'credits_per_month': Decimal(100), + 'remaining_credits': Decimal(0), + '_month_of_last_credit_reset': '0', + 'application_status': 'APPROVED', + } + ) costs = [ { @@ -413,17 +439,7 @@ def test_put_jobs_costs(tables): assert jobs[7]['credit_cost'] == Decimal('0.4') assert tables.jobs_table.scan()['Items'] == sorted(jobs, key=lambda item: item['job_id']) - assert tables.users_table.scan()['Items'] == [{'user_id': 'user1', 'remaining_credits': Decimal('11.7')}] - - -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'] == sorted(jobs, key=lambda item: item['job_id']) - assert tables.users_table.scan()['Items'] == [{'user_id': 'user1', 'remaining_credits': 3}] + assert tables.users_table.scan()['Items'][0]['remaining_credits'] == Decimal('11.7') def test_put_jobs_insufficient_credits(tables, monkeypatch): From 7cdc87f87ff3033de835fbb9451374a7389696f7 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 3 May 2024 14:47:24 -0800 Subject: [PATCH 40/84] Update `approved_user` fixture --- tests/conftest.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 574174351..a257def2b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,4 @@ +from decimal import Decimal from os import environ, path import pytest @@ -45,9 +46,9 @@ class Tables: def approved_user(tables): user = { 'user_id': 'approved_user', - 'remaining_credits': 10000, + 'remaining_credits': Decimal(0), + '_month_of_last_credit_reset': '0', 'application_status': 'APPROVED', - '_month_of_last_credit_reset': '2024-01-01', } tables.users_table.put_item(Item=user) return user From 15aee26ef9afad6ab85c622890e775f41aa191bd Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 3 May 2024 14:52:43 -0800 Subject: [PATCH 41/84] Update tests/conftest.py --- tests/conftest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index a257def2b..1900bec80 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -47,7 +47,6 @@ def approved_user(tables): user = { 'user_id': 'approved_user', 'remaining_credits': Decimal(0), - '_month_of_last_credit_reset': '0', 'application_status': 'APPROVED', } tables.users_table.put_item(Item=user) From 09ea8776f0be68afb21ab3ed574fff4928d7b0bb Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 3 May 2024 15:11:43 -0800 Subject: [PATCH 42/84] make `_month_of_last_credit_reset` optional --- lib/dynamo/dynamo/user.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index f84b43219..f789663c4 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -91,12 +91,7 @@ def _get_current_month() -> str: def _create_user(user_id: str, users_table) -> dict: - user = { - 'user_id': user_id, - 'remaining_credits': Decimal(0), - '_month_of_last_credit_reset': '0', - 'application_status': APPLICATION_NOT_STARTED - } + user = {'user_id': user_id, 'remaining_credits': Decimal(0), 'application_status': APPLICATION_NOT_STARTED} try: users_table.put_item(Item=user, ConditionExpression='attribute_not_exists(user_id)') except botocore.exceptions.ClientError as e: @@ -107,9 +102,10 @@ def _create_user(user_id: str, users_table) -> dict: def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> dict: + # TODO replace put_item with update_item? if ( user['application_status'] == APPLICATION_APPROVED - and user['_month_of_last_credit_reset'] < current_month # noqa: W503 + and user.get('_month_of_last_credit_reset', '0') < current_month # noqa: W503 and user['remaining_credits'] is not None # noqa: W503 ): user['_month_of_last_credit_reset'] = current_month @@ -119,7 +115,8 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month Item=user, ConditionExpression=( 'application_status = :approved' - ' AND #month_of_last_credit_reset < :current_month' + ' AND (attribute_not_exists(#month_of_last_credit_reset)' + ' OR #month_of_last_credit_reset < :current_month)' ' AND attribute_type(remaining_credits, :number)' ), ExpressionAttributeNames={'#month_of_last_credit_reset': '_month_of_last_credit_reset'}, @@ -136,7 +133,7 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month ) raise elif user['application_status'] != APPLICATION_APPROVED and user['remaining_credits'] != Decimal(0): - user['_month_of_last_credit_reset'] = '0' + del user['_month_of_last_credit_reset'] user['remaining_credits'] = Decimal(0) try: users_table.put_item( From 4c0db59e0b5b1638e9cf3ffd6db139141dd3228a Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 3 May 2024 15:20:47 -0800 Subject: [PATCH 43/84] pop --- 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 f789663c4..2fc050589 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -133,7 +133,7 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month ) raise elif user['application_status'] != APPLICATION_APPROVED and user['remaining_credits'] != Decimal(0): - del user['_month_of_last_credit_reset'] + user.pop('_month_of_last_credit_reset', None) user['remaining_credits'] = Decimal(0) try: users_table.put_item( From a4b3b2538f58a444c7206d8948b29ba1959daf70 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 3 May 2024 15:23:13 -0800 Subject: [PATCH 44/84] only return user id string from approved_user fixture --- tests/conftest.py | 4 ++-- tests/test_api/test_submit_job.py | 26 +++++++++++++------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 1900bec80..5a6a948ae 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -43,14 +43,14 @@ class Tables: @pytest.fixture -def approved_user(tables): +def approved_user(tables) -> str: user = { 'user_id': 'approved_user', 'remaining_credits': Decimal(0), 'application_status': 'APPROVED', } tables.users_table.put_item(Item=user) - return user + return user['user_id'] def list_have_same_elements(l1, l2): diff --git a/tests/test_api/test_submit_job.py b/tests/test_api/test_submit_job.py index cc392f140..40690dc76 100644 --- a/tests/test_api/test_submit_job.py +++ b/tests/test_api/test_submit_job.py @@ -7,7 +7,7 @@ def test_submit_one_job(client, approved_user): - login(client, username=approved_user['user_id']) + login(client, username=approved_user) batch = [make_job()] setup_requests_mock(batch) response = submit_batch(client, batch) @@ -16,11 +16,11 @@ def test_submit_one_job(client, approved_user): assert len(jobs) == 1 assert jobs[0]['status_code'] == 'PENDING' assert jobs[0]['request_time'] <= format_time(datetime.now(timezone.utc)) - assert jobs[0]['user_id'] == approved_user['user_id'] + assert jobs[0]['user_id'] == approved_user def test_submit_insar_gamma(client, approved_user): - login(client, username=approved_user['user_id']) + login(client, username=approved_user) granules = [ 'S1A_IW_SLC__1SDV_20200720T172109_20200720T172128_033541_03E2FB_341F', 'S1A_IW_SLC__1SDV_20200813T172110_20200813T172129_033891_03EE3F_2C3E', @@ -56,7 +56,7 @@ def test_submit_insar_gamma(client, approved_user): def test_submit_autorift(client, approved_user): - login(client, username=approved_user['user_id']) + login(client, username=approved_user) job = make_job( [ 'S1A_IW_SLC__1SDV_20200720T172109_20200720T172128_033541_03E2FB_341F', @@ -71,7 +71,7 @@ def test_submit_autorift(client, approved_user): def test_submit_multiple_job_types(client, approved_user): - login(client, username=approved_user['user_id']) + login(client, username=approved_user) rtc_gamma_job = make_job() insar_gamma_job = make_job( [ @@ -95,7 +95,7 @@ def test_submit_multiple_job_types(client, approved_user): def test_submit_many_jobs(client, approved_user): max_jobs = 25 - login(client, username=approved_user['user_id']) + login(client, username=approved_user) batch = [make_job() for ii in range(max_jobs)] setup_requests_mock(batch) @@ -113,7 +113,7 @@ def test_submit_many_jobs(client, approved_user): def test_submit_exceeds_remaining_credits(client, approved_user, monkeypatch): - login(client, username=approved_user['user_id']) + login(client, username=approved_user) monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') batch1 = [make_job() for _ in range(20)] @@ -138,7 +138,7 @@ def test_submit_without_jobs(client): def test_submit_job_without_name(client, approved_user): - login(client, username=approved_user['user_id']) + login(client, username=approved_user) batch = [ make_job(name=None) ] @@ -199,7 +199,7 @@ def test_submit_job_granule_does_not_exist(client, tables): def test_submit_good_rtc_granule_names(client, approved_user): - login(client, username=approved_user['user_id']) + login(client, username=approved_user) good_granule_names = [ 'S1B_IW_SLC__1SDV_20200604T082207_20200604T082234_021881_029874_5E38', 'S1A_IW_SLC__1SSH_20150608T205059_20150608T205126_006287_0083E8_C4F0', @@ -253,7 +253,7 @@ def test_submit_bad_rtc_granule_names(client): def test_submit_good_autorift_granule_names(client, approved_user): - login(client, username=approved_user['user_id']) + login(client, username=approved_user) good_granule_names = [ 'S1B_IW_SLC__1SDV_20200604T082207_20200604T082234_021881_029874_5E38', 'S1A_IW_SLC__1SSH_20150608T205059_20150608T205126_006287_0083E8_C4F0', @@ -319,7 +319,7 @@ def test_submit_bad_autorift_granule_names(client): def test_submit_mixed_job_parameters(client, approved_user): - login(client, username=approved_user['user_id']) + login(client, username=approved_user) rtc_parameters = { 'resolution': 30.0, @@ -376,7 +376,7 @@ def test_submit_mixed_job_parameters(client, approved_user): def test_float_input(client, approved_user): - login(client, username=approved_user['user_id']) + login(client, username=approved_user) batch = [make_job(parameters={'resolution': 30.0})] setup_requests_mock(batch) response = submit_batch(client, batch) @@ -391,7 +391,7 @@ def test_float_input(client, approved_user): def test_submit_validate_only(client, tables, approved_user): - login(client, username=approved_user['user_id']) + login(client, username=approved_user) batch = [make_job()] setup_requests_mock(batch) From 7868359e0428e61e3c8cb82ed160f69ee2afef4b Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 3 May 2024 15:39:44 -0800 Subject: [PATCH 45/84] reset credits using update_item --- lib/dynamo/dynamo/user.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 2fc050589..ad8d81a31 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -102,17 +102,15 @@ def _create_user(user_id: str, users_table) -> dict: def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month: str, users_table) -> dict: - # TODO replace put_item with update_item? if ( user['application_status'] == APPLICATION_APPROVED and user.get('_month_of_last_credit_reset', '0') < current_month # noqa: W503 and user['remaining_credits'] is not None # noqa: W503 ): - user['_month_of_last_credit_reset'] = current_month - user['remaining_credits'] = user.get('credits_per_month', default_credits) try: - users_table.put_item( - Item=user, + user = users_table.update_item( + Key={'user_id': user['user_id']}, + UpdateExpression='SET remaining_credits = :credits, #month_of_last_credit_reset = :current_month', ConditionExpression=( 'application_status = :approved' ' AND (attribute_not_exists(#month_of_last_credit_reset)' @@ -122,10 +120,12 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month ExpressionAttributeNames={'#month_of_last_credit_reset': '_month_of_last_credit_reset'}, ExpressionAttributeValues={ ':approved': APPLICATION_APPROVED, + ':credits': user.get('credits_per_month', default_credits), ':current_month': current_month, ':number': 'N', }, - ) + ReturnValues='ALL_NEW', + )['Attributes'] except botocore.exceptions.ClientError as e: if e.response['Error']['Code'] == 'ConditionalCheckFailedException': raise DatabaseConditionException( @@ -133,14 +133,15 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month ) raise elif user['application_status'] != APPLICATION_APPROVED and user['remaining_credits'] != Decimal(0): - user.pop('_month_of_last_credit_reset', None) - user['remaining_credits'] = Decimal(0) try: - users_table.put_item( - Item=user, + user = users_table.update_item( + Key={'user_id': user['user_id']}, + UpdateExpression='SET remaining_credits = :zero REMOVE #month_of_last_credit_reset', ConditionExpression='NOT (application_status = :approved) AND NOT (remaining_credits = :zero)', + ExpressionAttributeNames={'#month_of_last_credit_reset': '_month_of_last_credit_reset'}, ExpressionAttributeValues={':approved': APPLICATION_APPROVED, ':zero': Decimal(0)}, - ) + ReturnValues='ALL_NEW', + )['Attributes'] except botocore.exceptions.ClientError as e: if e.response['Error']['Code'] == 'ConditionalCheckFailedException': raise DatabaseConditionException( From b96ee1c8f984a2eb875bec3c8d5ac89a331a3b1e Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Fri, 3 May 2024 15:40:28 -0800 Subject: [PATCH 46/84] update dynamo job tests --- tests/test_dynamo/test_jobs.py | 87 ++++++++++++---------------------- 1 file changed, 31 insertions(+), 56 deletions(-) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 37de40eae..0341f89cf 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -257,21 +257,14 @@ def test_get_credit_cost_validate_keys(): dynamo.jobs._get_credit_cost({'job_type': 'JOB_TYPE_F'}, costs) -def test_put_jobs(tables): - tables.users_table.put_item( - Item={ - 'user_id': 'user1', - 'remaining_credits': Decimal(10), - '_month_of_last_credit_reset': '2024-02', - 'application_status': 'APPROVED', - } - ) +def test_put_jobs(tables, monkeypatch, approved_user): + 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) + jobs = dynamo.jobs.put_jobs(approved_user, payload) mock_get_current_month.assert_called_once_with() @@ -281,7 +274,7 @@ def test_put_jobs(tables): '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['user_id'] == approved_user assert job['status_code'] == 'PENDING' assert job['execution_started'] is False assert job['credit_cost'] == 1 @@ -289,22 +282,14 @@ def test_put_jobs(tables): assert tables.jobs_table.scan()['Items'] == sorted(jobs, key=lambda item: item['job_id']) assert tables.users_table.scan()['Items'] == [{ - 'user_id': 'user1', - 'remaining_credits': 7, + 'user_id': approved_user, + 'remaining_credits': Decimal(7), '_month_of_last_credit_reset': '2024-02', 'application_status': 'APPROVED', }] -def test_put_jobs_default_params(tables): - tables.users_table.put_item( - Item={ - 'user_id': 'user1', - 'remaining_credits': Decimal(0), - '_month_of_last_credit_reset': '0', - 'application_status': 'APPROVED', - } - ) +def test_put_jobs_default_params(tables, approved_user): default_params = { 'JOB_TYPE_A': {'a1': 'a1_default', 'a2': 'a2_default'}, 'JOB_TYPE_B': {'b1': 'b1_default'}, @@ -331,7 +316,7 @@ def test_put_jobs_default_params(tables): ] with unittest.mock.patch('dynamo.jobs.DEFAULT_PARAMS_BY_JOB_TYPE', default_params), \ unittest.mock.patch('dynamo.jobs.COSTS', costs): - jobs = dynamo.jobs.put_jobs('user1', payload) + jobs = dynamo.jobs.put_jobs(approved_user, payload) assert 'job_parameters' not in jobs[0] assert jobs[1]['job_parameters'] == {'a1': 'a1_default', 'a2': 'a2_default'} @@ -349,16 +334,8 @@ def test_put_jobs_default_params(tables): assert tables.jobs_table.scan()['Items'] == sorted(jobs, key=lambda item: item['job_id']) -def test_put_jobs_costs(tables): - tables.users_table.put_item( - Item={ - 'user_id': 'user1', - 'credits_per_month': Decimal(100), - 'remaining_credits': Decimal(0), - '_month_of_last_credit_reset': '0', - 'application_status': 'APPROVED', - } - ) +def test_put_jobs_costs(tables, monkeypatch, approved_user): + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '100') costs = [ { @@ -416,7 +393,7 @@ def test_put_jobs_costs(tables): ] with unittest.mock.patch('dynamo.jobs.COSTS', costs), \ unittest.mock.patch('dynamo.jobs.DEFAULT_PARAMS_BY_JOB_TYPE', default_params): - jobs = dynamo.jobs.put_jobs('user1', payload) + jobs = dynamo.jobs.put_jobs(approved_user, payload) assert len(jobs) == 8 @@ -442,26 +419,22 @@ def test_put_jobs_costs(tables): assert tables.users_table.scan()['Items'][0]['remaining_credits'] == Decimal('11.7') -def test_put_jobs_insufficient_credits(tables, monkeypatch): +def test_put_jobs_insufficient_credits(tables, monkeypatch, approved_user): monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '1') payload = [{'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' - with pytest.raises(dynamo.jobs.InsufficientCreditsError): - dynamo.jobs.put_jobs('user1', payload) + with pytest.raises(dynamo.jobs.InsufficientCreditsError): + dynamo.jobs.put_jobs(approved_user, 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'} - ] + assert tables.users_table.scan()['Items'][0]['remaining_credits'] == 1 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', 'remaining_credits': None}) + tables.users_table.put_item(Item={'user_id': 'user1', 'remaining_credits': None, 'application_status': 'APPROVED'}) jobs = dynamo.jobs.put_jobs('user1', payload) @@ -472,7 +445,8 @@ def test_put_jobs_infinite_credits(tables, monkeypatch): def test_put_jobs_priority_override(tables): payload = [{'name': 'name1'}, {'name': 'name2'}] - tables.users_table.put_item(Item={'user_id': 'user1', 'priority_override': 100, 'remaining_credits': 3}) + user = {'user_id': 'user1', 'priority_override': 100, 'remaining_credits': 3, 'application_status': 'APPROVED'} + tables.users_table.put_item(Item=user) jobs = dynamo.jobs.put_jobs('user1', payload) @@ -480,7 +454,8 @@ 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_override': 550, 'remaining_credits': None}) + user = {'user_id': 'user1', 'priority_override': 550, 'remaining_credits': None, 'application_status': 'APPROVED'} + tables.users_table.put_item(Item=user) jobs = dynamo.jobs.put_jobs('user1', payload) @@ -489,31 +464,31 @@ def test_put_jobs_priority_override(tables): assert job['priority'] == 550 -def test_put_jobs_priority(tables): - tables.users_table.put_item(Item={'user_id': 'user1', 'remaining_credits': 7}) +def test_put_jobs_priority(tables, monkeypatch, approved_user): + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '7') - jobs = dynamo.jobs.put_jobs(user_id='user1', jobs=[{}, {}, {}]) + jobs = dynamo.jobs.put_jobs(user_id=approved_user, jobs=[{}, {}, {}]) 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=[{}, {}, {}, {}])) + jobs.extend(dynamo.jobs.put_jobs(user_id=approved_user, 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): - tables.users_table.put_item(Item={'user_id': 'user1', 'remaining_credits': 10_003}) +def test_put_jobs_priority_extra_credits(tables, monkeypatch, approved_user): + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '10003') - jobs = dynamo.jobs.put_jobs(user_id='user1', jobs=[{}]) + jobs = dynamo.jobs.put_jobs(user_id=approved_user, jobs=[{}]) assert jobs[0]['priority'] == 9999 - jobs.extend(dynamo.jobs.put_jobs(user_id='user1', jobs=[{}])) + jobs.extend(dynamo.jobs.put_jobs(user_id=approved_user, jobs=[{}])) assert jobs[1]['priority'] == 9999 - jobs.extend(dynamo.jobs.put_jobs(user_id='user1', jobs=[{}] * 6)) + jobs.extend(dynamo.jobs.put_jobs(user_id=approved_user, jobs=[{}] * 6)) assert jobs[2]['priority'] == 9999 assert jobs[3]['priority'] == 9999 assert jobs[4]['priority'] == 9999 @@ -522,11 +497,11 @@ def test_put_jobs_priority_extra_credits(tables): assert jobs[7]['priority'] == 9996 -def test_put_jobs_decrement_credits_failure(tables): +def test_put_jobs_decrement_credits_failure(tables, approved_user): 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('user1', [{'name': 'job1'}]) + dynamo.jobs.put_jobs(approved_user, [{'name': 'job1'}]) assert tables.jobs_table.scan()['Items'] == [] From 73cb406afb23c7bd9f4a01108c5b689fb55ada44 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Fri, 3 May 2024 15:45:53 -0800 Subject: [PATCH 47/84] use weird `<>` operator --- 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 ad8d81a31..8d8526279 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -137,7 +137,7 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month user = users_table.update_item( Key={'user_id': user['user_id']}, UpdateExpression='SET remaining_credits = :zero REMOVE #month_of_last_credit_reset', - ConditionExpression='NOT (application_status = :approved) AND NOT (remaining_credits = :zero)', + ConditionExpression='application_status <> :approved AND remaining_credits <> :zero', ExpressionAttributeNames={'#month_of_last_credit_reset': '_month_of_last_credit_reset'}, ExpressionAttributeValues={':approved': APPLICATION_APPROVED, ':zero': Decimal(0)}, ReturnValues='ALL_NEW', From 1e6e73f3122b256d0524822cfd71cb263ab8e3be Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 6 May 2024 09:21:12 -0800 Subject: [PATCH 48/84] fix all but 1 failing dynamo.user tests --- tests/test_dynamo/test_user.py | 100 +++++++++++++++++++++++---------- 1 file changed, 69 insertions(+), 31 deletions(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index e487911af..c75a6aeea 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -5,6 +5,7 @@ import pytest import dynamo.user +from dynamo.user import APPLICATION_APPROVED, APPLICATION_NOT_STARTED def test_get_or_create_user_reset(tables, monkeypatch): @@ -51,14 +52,11 @@ def test_get_or_create_user_create(tables, monkeypatch): 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 - ) + user = dynamo.user._create_user(user_id='foo', users_table=tables.users_table) - assert user == {'user_id': 'foo', 'remaining_credits': Decimal(25), 'month_of_last_credits_reset': '2024-02'} + assert user == { + 'user_id': 'foo', 'remaining_credits': Decimal(0), 'application_status': APPLICATION_NOT_STARTED + } assert tables.users_table.scan()['Items'] == [user] @@ -66,19 +64,16 @@ def test_create_user_already_exists(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 - ) + dynamo.user._create_user(user_id='foo', users_table=tables.users_table) assert tables.users_table.scan()['Items'] == [{'user_id': 'foo'}] def test_reset_credits(tables, monkeypatch): original_user_record = { - 'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-01' + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + 'application_status': APPLICATION_APPROVED, } tables.users_table.put_item(Item=original_user_record) @@ -89,7 +84,12 @@ def test_reset_credits(tables, monkeypatch): users_table=tables.users_table, ) - assert user == {'user_id': 'foo', 'remaining_credits': Decimal(25), 'month_of_last_credits_reset': '2024-02'} + assert user == { + 'user_id': 'foo', + 'remaining_credits': Decimal(25), + '_month_of_last_credit_reset': '2024-02', + 'application_status': APPLICATION_APPROVED, + } assert tables.users_table.scan()['Items'] == [user] @@ -98,7 +98,7 @@ def test_reset_credits_override(tables, monkeypatch): 'user_id': 'foo', 'remaining_credits': Decimal(10), 'credits_per_month': Decimal(50), - 'month_of_last_credits_reset': '2024-01', + 'application_status': APPLICATION_APPROVED, } tables.users_table.put_item(Item=original_user_record) @@ -113,14 +113,18 @@ def test_reset_credits_override(tables, monkeypatch): 'user_id': 'foo', 'remaining_credits': Decimal(50), 'credits_per_month': Decimal(50), - 'month_of_last_credits_reset': '2024-02', + '_month_of_last_credit_reset': '2024-02', + 'application_status': APPLICATION_APPROVED, } assert tables.users_table.scan()['Items'] == [user] def test_reset_credits_same_month(tables, monkeypatch): original_user_record = { - 'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-02' + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + '_month_of_last_credit_reset': '2024-02', + 'application_status': APPLICATION_APPROVED, } tables.users_table.put_item(Item=original_user_record) @@ -131,13 +135,20 @@ def test_reset_credits_same_month(tables, monkeypatch): users_table=tables.users_table, ) - assert user == {'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-02'} + assert user == { + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + '_month_of_last_credit_reset': '2024-02', + 'application_status': APPLICATION_APPROVED, + } assert tables.users_table.scan()['Items'] == [user] def test_reset_credits_infinite_credits(tables, monkeypatch): original_user_record = { - 'user_id': 'foo', 'remaining_credits': None, 'month_of_last_credits_reset': '2024-01' + 'user_id': 'foo', + 'remaining_credits': None, + 'application_status': APPLICATION_APPROVED, } tables.users_table.put_item(Item=original_user_record) @@ -148,44 +159,71 @@ def test_reset_credits_infinite_credits(tables, monkeypatch): users_table=tables.users_table, ) - assert user == {'user_id': 'foo', 'remaining_credits': None, 'month_of_last_credits_reset': '2024-01'} + assert user == { + 'user_id': 'foo', + 'remaining_credits': None, + 'application_status': APPLICATION_APPROVED, + } assert tables.users_table.scan()['Items'] == [user] def test_reset_credits_failed_same_month(tables, monkeypatch): tables.users_table.put_item( - Item={'user_id': 'foo', 'remaining_credits': Decimal(10), 'month_of_last_credits_reset': '2024-02'} + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + '_month_of_last_credit_reset': '2024-02', + 'application_status': APPLICATION_APPROVED, + } ) 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'}, + user={ + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + '_month_of_last_credit_reset': '2024-01', + 'application_status': APPLICATION_APPROVED, + }, default_credits=Decimal(25), current_month='2024-02', 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'} - ] + assert tables.users_table.scan()['Items'] == [{ + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + '_month_of_last_credit_reset': '2024-02', + 'application_status': APPLICATION_APPROVED, + }] def test_reset_credits_failed_infinite_credits(tables, monkeypatch): tables.users_table.put_item( - Item={'user_id': 'foo', 'remaining_credits': None, 'month_of_last_credits_reset': '2024-01'} + Item={ + 'user_id': 'foo', + 'remaining_credits': None, + 'application_status': APPLICATION_APPROVED, + } ) 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'}, + user={ + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + 'application_status': APPLICATION_APPROVED, + }, default_credits=Decimal(25), current_month='2024-02', users_table=tables.users_table, ) - assert tables.users_table.scan()['Items'] == [ - {'user_id': 'foo', 'remaining_credits': None, 'month_of_last_credits_reset': '2024-01'} - ] + assert tables.users_table.scan()['Items'] == [{ + 'user_id': 'foo', + 'remaining_credits': None, + 'application_status': APPLICATION_APPROVED, + }] def test_decrement_credits(tables): From 3fdcda46cbc7d8864e12df50426e5c62691ac73e Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 6 May 2024 09:24:22 -0800 Subject: [PATCH 49/84] fix last user test --- tests/test_dynamo/test_user.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index c75a6aeea..4956a6628 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -30,21 +30,14 @@ 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' +def test_get_or_create_user_create(tables): + with unittest.mock.patch('dynamo.user._create_user') as mock_create_user: 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, ) From 7f2783834401c12082a8953e9235de60a9d58887 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 6 May 2024 10:09:54 -0800 Subject: [PATCH 50/84] add test_put_jobs_application_status --- tests/test_dynamo/test_jobs.py | 49 ++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index 0341f89cf..f6789652b 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -289,6 +289,55 @@ def test_put_jobs(tables, monkeypatch, approved_user): }] +def test_put_jobs_application_status(tables): + payload = [{'name': 'name1'}, {'name': 'name1'}, {'name': 'name2'}] + + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': dynamo.user.APPLICATION_NOT_STARTED, + } + ) + with pytest.raises( + dynamo.jobs.UnapprovedUserError, match=r'^User foo has not yet applied for a monthly credit allotment.*'): + dynamo.jobs.put_jobs('foo', payload) + assert tables.jobs_table.scan()['Items'] == [] + + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': dynamo.user.APPLICATION_PENDING, + } + ) + with pytest.raises(dynamo.jobs.UnapprovedUserError, match=r'^User foo has a pending application.*'): + dynamo.jobs.put_jobs('foo', payload) + assert tables.jobs_table.scan()['Items'] == [] + + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': dynamo.user.APPLICATION_REJECTED, + } + ) + with pytest.raises(dynamo.jobs.UnapprovedUserError, match=r'.*application for user foo has been rejected.*'): + dynamo.jobs.put_jobs('foo', payload) + assert tables.jobs_table.scan()['Items'] == [] + + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': 'bar', + } + ) + with pytest.raises(ValueError, match=r'^User foo has an invalid application status: bar$'): + dynamo.jobs.put_jobs('foo', payload) + assert tables.jobs_table.scan()['Items'] == [] + + def test_put_jobs_default_params(tables, approved_user): default_params = { 'JOB_TYPE_A': {'a1': 'a1_default', 'a2': 'a2_default'}, From ef378ffaa4485d9ca56aecc6c0d8043e232e6687 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 6 May 2024 11:20:15 -0800 Subject: [PATCH 51/84] add new tests for reset credits --- tests/test_dynamo/test_user.py | 164 +++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 4956a6628..6118314fc 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -86,6 +86,31 @@ def test_reset_credits(tables, monkeypatch): assert tables.users_table.scan()['Items'] == [user] +def test_reset_credits_month_exists(tables, monkeypatch): + original_user_record = { + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + '_month_of_last_credit_reset': '2024-01', + 'application_status': APPLICATION_APPROVED, + } + 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_credit_reset': '2024-02', + 'application_status': APPLICATION_APPROVED, + } + assert tables.users_table.scan()['Items'] == [user] + + def test_reset_credits_override(tables, monkeypatch): original_user_record = { 'user_id': 'foo', @@ -160,6 +185,83 @@ def test_reset_credits_infinite_credits(tables, monkeypatch): assert tables.users_table.scan()['Items'] == [user] +def test_reset_credits_to_zero(tables): + original_user_record = { + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + '_month_of_last_credit_reset': '2024-02', + 'application_status': 'bar', + } + 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(0), + 'application_status': 'bar', + } + assert tables.users_table.scan()['Items'] == [user] + + +def test_reset_credits_already_at_zero(tables): + original_user_record = { + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + '_month_of_last_credit_reset': '2024-02', + 'application_status': 'bar', + } + 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(0), + '_month_of_last_credit_reset': '2024-02', + 'application_status': 'bar', + } + assert tables.users_table.scan()['Items'] == [user] + + +def test_reset_credits_failed_not_approved(tables): + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + 'application_status': 'bar', + } + ) + + with pytest.raises(dynamo.user.DatabaseConditionException): + dynamo.user._reset_credits_if_needed( + user={ + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + 'application_status': APPLICATION_APPROVED, + }, + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table, + ) + + assert tables.users_table.scan()['Items'] == [{ + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + 'application_status': 'bar', + }] + + def test_reset_credits_failed_same_month(tables, monkeypatch): tables.users_table.put_item( Item={ @@ -219,6 +321,68 @@ def test_reset_credits_failed_infinite_credits(tables, monkeypatch): }] +def test_reset_credits_failed_approved(tables): + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + '_month_of_last_credit_reset': '2024-02', + 'application_status': APPLICATION_APPROVED, + } + ) + + with pytest.raises(dynamo.user.DatabaseConditionException): + dynamo.user._reset_credits_if_needed( + user={ + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + '_month_of_last_credit_reset': '2024-02', + 'application_status': 'bar', + }, + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table, + ) + + assert tables.users_table.scan()['Items'] == [{ + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + '_month_of_last_credit_reset': '2024-02', + 'application_status': APPLICATION_APPROVED, + }] + + +def test_reset_credits_failed_zero_credits(tables): + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + '_month_of_last_credit_reset': '2024-02', + 'application_status': 'bar', + } + ) + + with pytest.raises(dynamo.user.DatabaseConditionException): + dynamo.user._reset_credits_if_needed( + user={ + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + '_month_of_last_credit_reset': '2024-02', + 'application_status': 'bar', + }, + default_credits=Decimal(25), + current_month='2024-02', + users_table=tables.users_table, + ) + + assert tables.users_table.scan()['Items'] == [{ + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + '_month_of_last_credit_reset': '2024-02', + 'application_status': 'bar', + }] + + def test_decrement_credits(tables): tables.users_table.put_item(Item={'user_id': 'foo', 'remaining_credits': Decimal(25)}) From 405cd778f1bd89a950bb7b577c5c9c2550687c9d Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 6 May 2024 11:32:50 -0800 Subject: [PATCH 52/84] factor out private _update_user function --- lib/dynamo/dynamo/user.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 8d8526279..2c124309c 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -24,6 +24,11 @@ class ApplicationClosedError(Exception): def update_user(user_id: str, edl_access_token: str, body: dict) -> dict: user = get_or_create_user(user_id) + return _update_user(user, edl_access_token, body) + + +def _update_user(user: dict, edl_access_token: str, body: dict) -> dict: + user_id = user['user_id'] application_status = user['application_status'] if application_status in (APPLICATION_NOT_STARTED, APPLICATION_PENDING): edl_profile = _get_edl_profile(user_id, edl_access_token) From 2cb34b15ecf721ed46e5160e0d5677757bbe1d85 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 May 2024 19:44:23 +0000 Subject: [PATCH 53/84] Bump cfn-lint from 0.86.3 to 0.87.1 Bumps [cfn-lint](https://github.com/aws-cloudformation/cfn-python-lint) from 0.86.3 to 0.87.1. - [Release notes](https://github.com/aws-cloudformation/cfn-python-lint/releases) - [Changelog](https://github.com/aws-cloudformation/cfn-lint/blob/main/CHANGELOG.md) - [Commits](https://github.com/aws-cloudformation/cfn-python-lint/compare/v0.86.3...v0.87.1) --- updated-dependencies: - dependency-name: cfn-lint dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- requirements-all.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-all.txt b/requirements-all.txt index 6bb9a7f51..a96e555a8 100644 --- a/requirements-all.txt +++ b/requirements-all.txt @@ -17,4 +17,4 @@ flake8-blind-except==0.2.1 flake8-builtins==2.5.0 setuptools==69.5.1 openapi-spec-validator==0.7.1 -cfn-lint==0.86.3 +cfn-lint==0.87.1 From 6c7f4afffd906cbc1f7c38725f47946a13214bc7 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 May 2024 19:51:33 +0000 Subject: [PATCH 54/84] Bump actions/checkout from 4.1.2 to 4.1.5 Bumps [actions/checkout](https://github.com/actions/checkout) from 4.1.2 to 4.1.5. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v4.1.2...v4.1.5) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- .github/workflows/deploy-daac.yml | 2 +- .github/workflows/deploy-enterprise-test.yml | 2 +- .github/workflows/deploy-enterprise.yml | 2 +- .github/workflows/static-analysis.yml | 10 +++++----- .github/workflows/tests.yml | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/deploy-daac.yml b/.github/workflows/deploy-daac.yml index ee30d6f56..0a699c897 100644 --- a/.github/workflows/deploy-daac.yml +++ b/.github/workflows/deploy-daac.yml @@ -62,7 +62,7 @@ jobs: url: https://${{ matrix.domain }} steps: - - uses: actions/checkout@v4.1.2 + - uses: actions/checkout@v4.1.5 - uses: aws-actions/configure-aws-credentials@v4 with: diff --git a/.github/workflows/deploy-enterprise-test.yml b/.github/workflows/deploy-enterprise-test.yml index ec505a65c..822153f61 100644 --- a/.github/workflows/deploy-enterprise-test.yml +++ b/.github/workflows/deploy-enterprise-test.yml @@ -82,7 +82,7 @@ jobs: url: https://${{ matrix.domain }} steps: - - uses: actions/checkout@v4.1.2 + - uses: actions/checkout@v4.1.5 - uses: aws-actions/configure-aws-credentials@v4 with: diff --git a/.github/workflows/deploy-enterprise.yml b/.github/workflows/deploy-enterprise.yml index b3ef70364..b92e8cfd1 100644 --- a/.github/workflows/deploy-enterprise.yml +++ b/.github/workflows/deploy-enterprise.yml @@ -229,7 +229,7 @@ jobs: url: https://${{ matrix.domain }} steps: - - uses: actions/checkout@v4.1.2 + - uses: actions/checkout@v4.1.5 - uses: aws-actions/configure-aws-credentials@v4 with: diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 3852a2b1c..ee35bf19f 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -6,7 +6,7 @@ jobs: flake8: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4.1.2 + - uses: actions/checkout@v4.1.5 - uses: actions/setup-python@v5 with: python-version: 3.9 @@ -23,7 +23,7 @@ jobs: matrix: security_environment: [ASF, EDC, JPL, JPL-public] steps: - - uses: actions/checkout@v4.1.2 + - uses: actions/checkout@v4.1.5 - uses: actions/setup-python@v5 with: python-version: 3.9 @@ -37,7 +37,7 @@ jobs: openapi-spec-validator: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4.1.2 + - uses: actions/checkout@v4.1.5 - uses: actions/setup-python@v5 with: python-version: 3.9 @@ -50,7 +50,7 @@ jobs: statelint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4.1.2 + - uses: actions/checkout@v4.1.5 - uses: ruby/setup-ruby@v1 with: ruby-version: 2.7 @@ -70,7 +70,7 @@ jobs: snyk: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4.1.2 + - uses: actions/checkout@v4.1.5 - uses: snyk/actions/setup@0.4.0 - uses: actions/setup-python@v5 with: diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 3e11a6a7f..a48a05477 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -7,7 +7,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4.1.2 + - uses: actions/checkout@v4.1.5 - uses: actions/setup-python@v5 with: From ef08ccfadc405868161eedff517bb8f7bde67043 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 6 May 2024 15:05:10 -0800 Subject: [PATCH 55/84] add tests for update_user --- tests/test_dynamo/test_user.py | 148 ++++++++++++++++++++++++++++++++- 1 file changed, 147 insertions(+), 1 deletion(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 6118314fc..bfcdc800b 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -5,7 +5,153 @@ import pytest import dynamo.user -from dynamo.user import APPLICATION_APPROVED, APPLICATION_NOT_STARTED +from dynamo.user import APPLICATION_APPROVED, APPLICATION_NOT_STARTED, APPLICATION_PENDING, APPLICATION_REJECTED + + +def test_update_user(tables): + with unittest.mock.patch('dynamo.user._get_edl_profile') as mock_get_edl_profile: + mock_get_edl_profile.return_value = {'key': 'value'} + user = dynamo.user.update_user( + 'foo', + 'test-edl-access-token', + {'use_case': 'I want data.'}, + ) + mock_get_edl_profile.assert_called_once_with('foo', 'test-edl-access-token') + + assert user == { + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': APPLICATION_PENDING, + '_edl_profile': {'key': 'value'}, + 'use_case': 'I want data.', + } + assert tables.users_table.scan()['Items'] == [user] + + +def test_update_user_not_started(tables): + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(5), + 'application_status': APPLICATION_NOT_STARTED, + } + ) + with unittest.mock.patch('dynamo.user._get_edl_profile') as mock_get_edl_profile: + mock_get_edl_profile.return_value = {'key': 'value'} + user = dynamo.user.update_user( + 'foo', + 'test-edl-access-token', + {'use_case': 'I want data.'}, + ) + mock_get_edl_profile.assert_called_once_with('foo', 'test-edl-access-token') + + assert user == { + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': APPLICATION_PENDING, + '_edl_profile': {'key': 'value'}, + 'use_case': 'I want data.', + } + assert tables.users_table.scan()['Items'] == [user] + + +def test_update_user_pending(tables): + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(5), + 'application_status': APPLICATION_PENDING, + '_edl_profile': {'key': 'old_value'}, + 'use_case': 'Old use case.', + } + ) + with unittest.mock.patch('dynamo.user._get_edl_profile') as mock_get_edl_profile: + mock_get_edl_profile.return_value = {'key': 'new_value'} + user = dynamo.user.update_user( + 'foo', + 'test-edl-access-token', + {'use_case': 'New use case.'}, + ) + mock_get_edl_profile.assert_called_once_with('foo', 'test-edl-access-token') + + assert user == { + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': APPLICATION_PENDING, + '_edl_profile': {'key': 'new_value'}, + 'use_case': 'New use case.', + } + assert tables.users_table.scan()['Items'] == [user] + + +def test_update_user_rejected(tables): + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(5), + 'application_status': APPLICATION_REJECTED, + } + ) + with pytest.raises(dynamo.user.ApplicationClosedError, match=r'.*application for user foo has been rejected.*'): + dynamo.user.update_user( + 'foo', + 'test-edl-access-token', + {'use_case': 'I want data.'}, + ) + assert tables.users_table.scan()['Items'] == [{ + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': APPLICATION_REJECTED, + }] + + +def test_update_user_approved(tables, monkeypatch): + monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(10), + 'application_status': APPLICATION_APPROVED, + } + ) + 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.user.ApplicationClosedError, match=r'^The application for user foo has already been approved.$'): + dynamo.user.update_user( + 'foo', + 'test-edl-access-token', + {'use_case': 'I want data.'}, + ) + mock_get_current_month.assert_called_once_with() + + assert tables.users_table.scan()['Items'] == [{ + 'user_id': 'foo', + 'remaining_credits': Decimal(25), + '_month_of_last_credit_reset': '2024-02', + 'application_status': APPLICATION_APPROVED, + }] + + +def test_update_user_invalid_status(tables): + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(5), + 'application_status': 'bar', + } + ) + with pytest.raises(ValueError, match=r'^User foo has an invalid application status: bar$'): + dynamo.user.update_user( + 'foo', + 'test-edl-access-token', + {'use_case': 'I want data.'}, + ) + assert tables.users_table.scan()['Items'] == [{ + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': 'bar', + }] def test_get_or_create_user_reset(tables, monkeypatch): From f9167484a72ca36cdf8272b271a2d1ca057c71ee Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 6 May 2024 15:24:48 -0800 Subject: [PATCH 56/84] test update_user condition expression --- tests/test_dynamo/test_user.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index bfcdc800b..a24af1da9 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -154,6 +154,35 @@ def test_update_user_invalid_status(tables): }] +def test_update_user_failed_application_status(tables): + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'application_status': 'bar', + } + ) + with unittest.mock.patch('dynamo.user.get_or_create_user') as mock_get_or_create_user, \ + unittest.mock.patch('dynamo.user._get_edl_profile') as mock_get_edl_profile: + mock_get_or_create_user.return_value = { + 'user_id': 'foo', + 'application_status': APPLICATION_NOT_STARTED, + } + mock_get_edl_profile.return_value = {'key': 'new_value'} + with pytest.raises(dynamo.user.DatabaseConditionException): + dynamo.user.update_user( + 'foo', + 'test-edl-access-token', + {'use_case': 'New use case.'}, + ) + mock_get_or_create_user.assert_called_once_with('foo') + mock_get_edl_profile.assert_called_once_with('foo', 'test-edl-access-token') + + assert tables.users_table.scan()['Items'] == [{ + 'user_id': 'foo', + 'application_status': 'bar', + }] + + def test_get_or_create_user_reset(tables, monkeypatch): monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') tables.users_table.put_item(Item={'user_id': 'foo'}) From fc064c350147f7f0ea3fcce5ec0b6ea23c908e86 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Mon, 6 May 2024 15:26:18 -0800 Subject: [PATCH 57/84] combine update_user and _update_user --- lib/dynamo/dynamo/user.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 2c124309c..8d8526279 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -24,11 +24,6 @@ class ApplicationClosedError(Exception): def update_user(user_id: str, edl_access_token: str, body: dict) -> dict: user = get_or_create_user(user_id) - return _update_user(user, edl_access_token, body) - - -def _update_user(user: dict, edl_access_token: str, body: dict) -> dict: - user_id = user['user_id'] application_status = user['application_status'] if application_status in (APPLICATION_NOT_STARTED, APPLICATION_PENDING): edl_profile = _get_edl_profile(user_id, edl_access_token) From 0ee90ee4ed304dc12e15e7498547f007435ccfc6 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 09:32:10 -0800 Subject: [PATCH 58/84] add test for `POST /jobs` 403 response --- tests/test_api/test_submit_job.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/test_api/test_submit_job.py b/tests/test_api/test_submit_job.py index 40690dc76..eb1272cd2 100644 --- a/tests/test_api/test_submit_job.py +++ b/tests/test_api/test_submit_job.py @@ -1,8 +1,10 @@ from datetime import datetime, timezone +from decimal import Decimal from http import HTTPStatus -from test_api.conftest import DEFAULT_USERNAME, login, make_job, setup_requests_mock, submit_batch +from test_api.conftest import login, make_job, setup_requests_mock, submit_batch +from dynamo.user import APPLICATION_PENDING from dynamo.util import format_time @@ -130,6 +132,24 @@ def test_submit_exceeds_remaining_credits(client, approved_user, monkeypatch): assert response2.json['detail'] == 'These jobs would cost 10.0 credits, but you have only 5.0 remaining.' +def test_submit_unapproved_user(client, tables): + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': APPLICATION_PENDING, + } + ) + login(client, username='foo') + + batch = [make_job()] + setup_requests_mock(batch) + + response = submit_batch(client, batch) + assert response.status_code == HTTPStatus.FORBIDDEN + assert response.json['detail'] == 'User foo has a pending application, please try again later.' + + def test_submit_without_jobs(client): login(client) batch = [] From 61cc90089d217240482776e913ee4e9fc9bb8e44 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 10:18:39 -0800 Subject: [PATCH 59/84] add tests for PATCH /user --- tests/test_api/test_patch_user.py | 67 +++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 tests/test_api/test_patch_user.py diff --git a/tests/test_api/test_patch_user.py b/tests/test_api/test_patch_user.py new file mode 100644 index 000000000..5ace5cdd5 --- /dev/null +++ b/tests/test_api/test_patch_user.py @@ -0,0 +1,67 @@ +import unittest.mock +from decimal import Decimal +from http import HTTPStatus + +from dynamo.user import APPLICATION_PENDING, APPLICATION_REJECTED +from test_api.conftest import DEFAULT_ACCESS_TOKEN, USER_URI, login + + +def test_patch_new_user(client, tables): + login(client, 'foo') + with unittest.mock.patch('dynamo.user._get_edl_profile') as mock_get_edl_profile: + mock_get_edl_profile.return_value = {} + response = client.patch(USER_URI, json={'use_case': 'I want data.'}) + mock_get_edl_profile.assert_called_once_with('foo', DEFAULT_ACCESS_TOKEN) + + assert response.status_code == HTTPStatus.OK + assert response.json == { + 'user_id': 'foo', + 'application_status': APPLICATION_PENDING, + 'remaining_credits': Decimal(0), + 'job_names': [], + 'use_case': 'I want data.', + } + + +def test_patch_pending_user(client, tables): + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(5), + 'application_status': APPLICATION_PENDING, + 'use_case': 'Old use case.', + '_edl_profile': {}, + '_foo': 'bar', + } + ) + + login(client, 'foo') + with unittest.mock.patch('dynamo.user._get_edl_profile') as mock_get_edl_profile: + mock_get_edl_profile.return_value = {} + response = client.patch(USER_URI, json={'use_case': 'New use case.'}) + mock_get_edl_profile.assert_called_once_with('foo', DEFAULT_ACCESS_TOKEN) + + assert response.status_code == HTTPStatus.OK + assert response.json == { + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': APPLICATION_PENDING, + 'use_case': 'New use case.', + 'job_names': [], + } + + +def test_patch_rejected_user(client, tables): + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': APPLICATION_REJECTED, + } + ) + + login(client, 'foo') + response = client.patch(USER_URI, json={'use_case': 'I want data.'}) + + assert response.status_code == HTTPStatus.FORBIDDEN + assert 'application for user foo has been rejected' in response.json['detail'] From c4a78c67d5d9ebc3b3dc353479130709cf7575e8 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 10:48:56 -0800 Subject: [PATCH 60/84] remove unused monkeypatch params --- tests/test_dynamo/test_user.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index a24af1da9..26ae0399f 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -237,7 +237,7 @@ def test_create_user_already_exists(tables): assert tables.users_table.scan()['Items'] == [{'user_id': 'foo'}] -def test_reset_credits(tables, monkeypatch): +def test_reset_credits(tables): original_user_record = { 'user_id': 'foo', 'remaining_credits': Decimal(10), @@ -261,7 +261,7 @@ def test_reset_credits(tables, monkeypatch): assert tables.users_table.scan()['Items'] == [user] -def test_reset_credits_month_exists(tables, monkeypatch): +def test_reset_credits_month_exists(tables): original_user_record = { 'user_id': 'foo', 'remaining_credits': Decimal(10), @@ -286,7 +286,7 @@ def test_reset_credits_month_exists(tables, monkeypatch): assert tables.users_table.scan()['Items'] == [user] -def test_reset_credits_override(tables, monkeypatch): +def test_reset_credits_override(tables): original_user_record = { 'user_id': 'foo', 'remaining_credits': Decimal(10), @@ -312,7 +312,7 @@ def test_reset_credits_override(tables, monkeypatch): assert tables.users_table.scan()['Items'] == [user] -def test_reset_credits_same_month(tables, monkeypatch): +def test_reset_credits_same_month(tables): original_user_record = { 'user_id': 'foo', 'remaining_credits': Decimal(10), @@ -337,7 +337,7 @@ def test_reset_credits_same_month(tables, monkeypatch): assert tables.users_table.scan()['Items'] == [user] -def test_reset_credits_infinite_credits(tables, monkeypatch): +def test_reset_credits_infinite_credits(tables): original_user_record = { 'user_id': 'foo', 'remaining_credits': None, @@ -437,7 +437,7 @@ def test_reset_credits_failed_not_approved(tables): }] -def test_reset_credits_failed_same_month(tables, monkeypatch): +def test_reset_credits_failed_same_month(tables): tables.users_table.put_item( Item={ 'user_id': 'foo', @@ -468,7 +468,7 @@ def test_reset_credits_failed_same_month(tables, monkeypatch): }] -def test_reset_credits_failed_infinite_credits(tables, monkeypatch): +def test_reset_credits_failed_infinite_credits(tables): tables.users_table.put_item( Item={ 'user_id': 'foo', From ed31b6847128b6f902d59a1ddd3b2d8af6c11d87 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 11:00:38 -0800 Subject: [PATCH 61/84] remove a TODO --- apps/api/src/hyp3_api/api-spec/openapi-spec.yml.j2 | 1 - 1 file changed, 1 deletion(-) 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 f583d5287..5cb8b6eab 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 @@ -313,7 +313,6 @@ components: use_case: description: Reason for wanting to use HyP3. type: string - # TODO maybe we should come up with a good example here example: I want to process data. user_id: From 00700d08bbeac3c102f90c27aa9644d165f1ab0f Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 11:51:28 -0800 Subject: [PATCH 62/84] add dynamo.exceptions module --- apps/api/src/hyp3_api/handlers.py | 7 +++-- lib/dynamo/dynamo/exceptions.py | 47 +++++++++++++++++++++++++++++++ lib/dynamo/dynamo/jobs.py | 34 ++++++++-------------- lib/dynamo/dynamo/user.py | 20 ++++--------- tests/test_dynamo/test_jobs.py | 18 ++++++++---- tests/test_dynamo/test_user.py | 33 +++++++++++++--------- 6 files changed, 99 insertions(+), 60 deletions(-) create mode 100644 lib/dynamo/dynamo/exceptions.py diff --git a/apps/api/src/hyp3_api/handlers.py b/apps/api/src/hyp3_api/handlers.py index 06d3b31ad..81c3afd13 100644 --- a/apps/api/src/hyp3_api/handlers.py +++ b/apps/api/src/hyp3_api/handlers.py @@ -4,6 +4,7 @@ from flask import abort, jsonify, request import dynamo +from dynamo.exceptions import InsufficientCreditsError, UnexpectedApplicationStatusError from hyp3_api import util from hyp3_api.validation import GranuleValidationError, validate_jobs @@ -32,9 +33,9 @@ def post_jobs(body, user): try: body['jobs'] = dynamo.jobs.put_jobs(user, body['jobs'], dry_run=body.get('validate_only')) - except dynamo.jobs.UnapprovedUserError as e: + except UnexpectedApplicationStatusError as e: abort(problem_format(403, str(e))) - except dynamo.jobs.InsufficientCreditsError as e: + except InsufficientCreditsError as e: abort(problem_format(400, str(e))) return body @@ -64,7 +65,7 @@ def patch_user(body: dict, user: str, edl_access_token: str) -> dict: print(body) try: user_record = dynamo.user.update_user(user, edl_access_token, body) - except dynamo.user.ApplicationClosedError as e: + except UnexpectedApplicationStatusError as e: abort(problem_format(403, str(e))) return _user_response(user_record) diff --git a/lib/dynamo/dynamo/exceptions.py b/lib/dynamo/dynamo/exceptions.py new file mode 100644 index 000000000..5c8377ee1 --- /dev/null +++ b/lib/dynamo/dynamo/exceptions.py @@ -0,0 +1,47 @@ +"""Custom exceptions for the dynamo library.""" + + +class DatabaseConditionException(Exception): + """Raised when a DynamoDB condition expression check fails.""" + + +class InsufficientCreditsError(Exception): + """Raised when trying to submit jobs whose total cost exceeds the user's remaining credits.""" + + +class InvalidApplicationStatusError(Exception): + """Raised for an invalid user application status.""" + + def __init__(self, user_id: str, application_status: str): + super().__init__(f'User {user_id} has an invalid application status: {application_status}') + + +class UnexpectedApplicationStatusError(Exception): + """Raised for an unexpected user application status.""" + + +class NotStartedApplicationError(UnexpectedApplicationStatusError): + def __init__(self, user_id: str): + # TODO replace with URL to the application form for the given deployment + super().__init__( + f'User {user_id} has not yet applied for a monthly credit allotment.' + ' Please visit to submit your application.' + ) + + +class PendingApplicationError(UnexpectedApplicationStatusError): + def __init__(self, user_id: str): + super().__init__(f'User {user_id} has a pending application, please try again later.') + + +class ApprovedApplicationError(UnexpectedApplicationStatusError): + def __init__(self, user_id: str): + super().__init__(f'The application for user {user_id} has already been approved.') + + +class RejectedApplicationError(UnexpectedApplicationStatusError): + def __init__(self, user_id: str): + super().__init__( + f'The application for user {user_id} has been rejected.' + ' For more information, please email ASF User Services at: uso@asf.alaska.edu' + ) diff --git a/lib/dynamo/dynamo/jobs.py b/lib/dynamo/dynamo/jobs.py index 2e646c04d..f95e26aca 100644 --- a/lib/dynamo/dynamo/jobs.py +++ b/lib/dynamo/dynamo/jobs.py @@ -9,6 +9,13 @@ from boto3.dynamodb.conditions import Attr, Key import dynamo.user +from dynamo.exceptions import ( + InsufficientCreditsError, + InvalidApplicationStatusError, + NotStartedApplicationError, + PendingApplicationError, + RejectedApplicationError, +) from dynamo.user import APPLICATION_APPROVED, APPLICATION_NOT_STARTED, APPLICATION_PENDING, APPLICATION_REJECTED from dynamo.util import DYNAMODB_RESOURCE, convert_floats_to_decimals, format_time, get_request_time_expression @@ -23,14 +30,6 @@ DEFAULT_PARAMS_BY_JOB_TYPE = {} -class UnapprovedUserError(Exception): - """Raised when the user is not approved for processing.""" - - -class InsufficientCreditsError(Exception): - """Raised when trying to submit jobs whose total cost exceeds the user's remaining credits.""" - - 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)) @@ -74,24 +73,13 @@ def put_jobs(user_id: str, jobs: List[dict], dry_run=False) -> List[dict]: def _raise_for_application_status(application_status: str, user_id: str) -> None: if application_status == APPLICATION_NOT_STARTED: - # TODO replace with URL to the application form for the given deployment - raise UnapprovedUserError( - f'User {user_id} has not yet applied for a monthly credit allotment.' - ' Please visit to submit your application.' - ) + raise NotStartedApplicationError(user_id) if application_status == APPLICATION_PENDING: - raise UnapprovedUserError( - f'User {user_id} has a pending application, please try again later.' - ) + raise PendingApplicationError(user_id) if application_status == APPLICATION_REJECTED: - raise UnapprovedUserError( - f'Unfortunately, the application for user {user_id} has been rejected.' - ' If you believe this was a mistake, please email ASF User Services at: uso@asf.alaska.edu' - ) + raise RejectedApplicationError(user_id) if application_status != APPLICATION_APPROVED: - raise ValueError( - f'User {user_id} has an invalid application status: {application_status}' - ) + raise InvalidApplicationStatusError(user_id, application_status) def _prepare_job_for_database( diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 8d8526279..549cb157a 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -6,6 +6,9 @@ import botocore.exceptions import requests +from dynamo.exceptions import ( + DatabaseConditionException, RejectedApplicationError, ApprovedApplicationError, InvalidApplicationStatusError +) from dynamo.util import DYNAMODB_RESOURCE APPLICATION_NOT_STARTED = 'NOT STARTED' @@ -14,14 +17,6 @@ APPLICATION_REJECTED = 'REJECTED' -class DatabaseConditionException(Exception): - """Raised when a DynamoDB condition expression check fails.""" - - -class ApplicationClosedError(Exception): - """Raised when the user attempts to update an application that has already been approved or rejected.""" - - def update_user(user_id: str, edl_access_token: str, body: dict) -> dict: user = get_or_create_user(user_id) application_status = user['application_status'] @@ -48,13 +43,10 @@ def update_user(user_id: str, edl_access_token: str, body: dict) -> dict: raise return user if application_status == APPLICATION_REJECTED: - raise ApplicationClosedError( - f'Unfortunately, the application for user {user_id} has been rejected.' - ' If you believe this was a mistake, please email ASF User Services at: uso@asf.alaska.edu' - ) + raise RejectedApplicationError(user_id) if application_status == APPLICATION_APPROVED: - raise ApplicationClosedError(f'The application for user {user_id} has already been approved.') - raise ValueError(f'User {user_id} has an invalid application status: {application_status}') + raise ApprovedApplicationError(user_id) + raise InvalidApplicationStatusError(user_id, application_status) def _get_edl_profile(user_id: str, edl_access_token: str) -> dict: diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index f6789652b..e19592651 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -6,6 +6,13 @@ from conftest import list_have_same_elements import dynamo +from dynamo.exceptions import ( + InsufficientCreditsError, + NotStartedApplicationError, + PendingApplicationError, + RejectedApplicationError, + InvalidApplicationStatusError, +) def test_query_jobs_by_user(tables): @@ -299,8 +306,7 @@ def test_put_jobs_application_status(tables): 'application_status': dynamo.user.APPLICATION_NOT_STARTED, } ) - with pytest.raises( - dynamo.jobs.UnapprovedUserError, match=r'^User foo has not yet applied for a monthly credit allotment.*'): + with pytest.raises(NotStartedApplicationError): dynamo.jobs.put_jobs('foo', payload) assert tables.jobs_table.scan()['Items'] == [] @@ -311,7 +317,7 @@ def test_put_jobs_application_status(tables): 'application_status': dynamo.user.APPLICATION_PENDING, } ) - with pytest.raises(dynamo.jobs.UnapprovedUserError, match=r'^User foo has a pending application.*'): + with pytest.raises(PendingApplicationError): dynamo.jobs.put_jobs('foo', payload) assert tables.jobs_table.scan()['Items'] == [] @@ -322,7 +328,7 @@ def test_put_jobs_application_status(tables): 'application_status': dynamo.user.APPLICATION_REJECTED, } ) - with pytest.raises(dynamo.jobs.UnapprovedUserError, match=r'.*application for user foo has been rejected.*'): + with pytest.raises(RejectedApplicationError): dynamo.jobs.put_jobs('foo', payload) assert tables.jobs_table.scan()['Items'] == [] @@ -333,7 +339,7 @@ def test_put_jobs_application_status(tables): 'application_status': 'bar', } ) - with pytest.raises(ValueError, match=r'^User foo has an invalid application status: bar$'): + with pytest.raises(InvalidApplicationStatusError): dynamo.jobs.put_jobs('foo', payload) assert tables.jobs_table.scan()['Items'] == [] @@ -472,7 +478,7 @@ def test_put_jobs_insufficient_credits(tables, monkeypatch, approved_user): monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '1') payload = [{'name': 'name1'}, {'name': 'name2'}] - with pytest.raises(dynamo.jobs.InsufficientCreditsError): + with pytest.raises(InsufficientCreditsError): dynamo.jobs.put_jobs(approved_user, payload) assert tables.jobs_table.scan()['Items'] == [] diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 26ae0399f..94cc12bd8 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -5,6 +5,12 @@ import pytest import dynamo.user +from dynamo.exceptions import ( + DatabaseConditionException, + RejectedApplicationError, + ApprovedApplicationError, + InvalidApplicationStatusError, +) from dynamo.user import APPLICATION_APPROVED, APPLICATION_NOT_STARTED, APPLICATION_PENDING, APPLICATION_REJECTED @@ -92,7 +98,7 @@ def test_update_user_rejected(tables): 'application_status': APPLICATION_REJECTED, } ) - with pytest.raises(dynamo.user.ApplicationClosedError, match=r'.*application for user foo has been rejected.*'): + with pytest.raises(RejectedApplicationError): dynamo.user.update_user( 'foo', 'test-edl-access-token', @@ -116,8 +122,7 @@ def test_update_user_approved(tables, monkeypatch): ) 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.user.ApplicationClosedError, match=r'^The application for user foo has already been approved.$'): + with pytest.raises(ApprovedApplicationError): dynamo.user.update_user( 'foo', 'test-edl-access-token', @@ -141,7 +146,7 @@ def test_update_user_invalid_status(tables): 'application_status': 'bar', } ) - with pytest.raises(ValueError, match=r'^User foo has an invalid application status: bar$'): + with pytest.raises(InvalidApplicationStatusError): dynamo.user.update_user( 'foo', 'test-edl-access-token', @@ -168,7 +173,7 @@ def test_update_user_failed_application_status(tables): 'application_status': APPLICATION_NOT_STARTED, } mock_get_edl_profile.return_value = {'key': 'new_value'} - with pytest.raises(dynamo.user.DatabaseConditionException): + with pytest.raises(DatabaseConditionException): dynamo.user.update_user( 'foo', 'test-edl-access-token', @@ -231,7 +236,7 @@ def test_create_user(tables): def test_create_user_already_exists(tables): tables.users_table.put_item(Item={'user_id': 'foo'}) - with pytest.raises(dynamo.user.DatabaseConditionException): + with pytest.raises(DatabaseConditionException): dynamo.user._create_user(user_id='foo', users_table=tables.users_table) assert tables.users_table.scan()['Items'] == [{'user_id': 'foo'}] @@ -418,7 +423,7 @@ def test_reset_credits_failed_not_approved(tables): } ) - with pytest.raises(dynamo.user.DatabaseConditionException): + with pytest.raises(DatabaseConditionException): dynamo.user._reset_credits_if_needed( user={ 'user_id': 'foo', @@ -447,7 +452,7 @@ def test_reset_credits_failed_same_month(tables): } ) - with pytest.raises(dynamo.user.DatabaseConditionException): + with pytest.raises(DatabaseConditionException): dynamo.user._reset_credits_if_needed( user={ 'user_id': 'foo', @@ -477,7 +482,7 @@ def test_reset_credits_failed_infinite_credits(tables): } ) - with pytest.raises(dynamo.user.DatabaseConditionException): + with pytest.raises(DatabaseConditionException): dynamo.user._reset_credits_if_needed( user={ 'user_id': 'foo', @@ -506,7 +511,7 @@ def test_reset_credits_failed_approved(tables): } ) - with pytest.raises(dynamo.user.DatabaseConditionException): + with pytest.raises(DatabaseConditionException): dynamo.user._reset_credits_if_needed( user={ 'user_id': 'foo', @@ -537,7 +542,7 @@ def test_reset_credits_failed_zero_credits(tables): } ) - with pytest.raises(dynamo.user.DatabaseConditionException): + with pytest.raises(DatabaseConditionException): dynamo.user._reset_credits_if_needed( user={ 'user_id': 'foo', @@ -586,7 +591,7 @@ def test_decrement_credits_invalid_cost(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): + with pytest.raises(DatabaseConditionException): dynamo.user.decrement_credits('foo', 2) assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': Decimal(1)}] @@ -595,7 +600,7 @@ def test_decrement_credits_cost_too_high(tables): assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': Decimal(0)}] - with pytest.raises(dynamo.user.DatabaseConditionException): + with pytest.raises(DatabaseConditionException): dynamo.user.decrement_credits('foo', 1) assert tables.users_table.scan()['Items'] == [{'user_id': 'foo', 'remaining_credits': Decimal(0)}] @@ -615,7 +620,7 @@ def test_decrement_credits_infinite_credits(tables): def test_decrement_credits_user_does_not_exist(tables): - with pytest.raises(dynamo.user.DatabaseConditionException): + with pytest.raises(DatabaseConditionException): dynamo.user.decrement_credits('foo', 1) assert tables.users_table.scan()['Items'] == [] From bba9f4ad678b2a7f3ff94b81a68016994f64e44a Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 12:00:47 -0800 Subject: [PATCH 63/84] don't hard-code application status values in tests --- tests/conftest.py | 4 +++- tests/test_api/test_get_user.py | 11 ++++++----- tests/test_dynamo/test_jobs.py | 18 ++++++++++++++---- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5a6a948ae..555c08616 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,6 +5,8 @@ import yaml from moto import mock_aws +from dynamo.user import APPLICATION_APPROVED + @pytest.fixture def table_properties(): @@ -47,7 +49,7 @@ def approved_user(tables) -> str: user = { 'user_id': 'approved_user', 'remaining_credits': Decimal(0), - 'application_status': 'APPROVED', + 'application_status': APPLICATION_APPROVED, } tables.users_table.put_item(Item=user) return user['user_id'] diff --git a/tests/test_api/test_get_user.py b/tests/test_api/test_get_user.py index df86b3261..debd8122a 100644 --- a/tests/test_api/test_get_user.py +++ b/tests/test_api/test_get_user.py @@ -1,6 +1,7 @@ from datetime import datetime, timezone from http import HTTPStatus +from dynamo.user import APPLICATION_APPROVED, APPLICATION_REJECTED, APPLICATION_NOT_STARTED from test_api.conftest import USER_URI, login, make_db_record from dynamo.util import format_time @@ -12,14 +13,14 @@ def test_get_new_user(client, tables, monkeypatch): assert response.status_code == HTTPStatus.OK assert response.json == { 'user_id': 'user', - 'application_status': 'NOT STARTED', + 'application_status': APPLICATION_NOT_STARTED, 'remaining_credits': 0, 'job_names': [], } def test_get_rejected_user(client, tables): - user = {'user_id': 'user', 'remaining_credits': 100, 'application_status': 'REJECTED'} + user = {'user_id': 'user', 'remaining_credits': 100, 'application_status': APPLICATION_REJECTED} tables.users_table.put_item(Item=user) login(client, 'user') @@ -27,7 +28,7 @@ def test_get_rejected_user(client, tables): assert response.status_code == HTTPStatus.OK assert response.json == { 'user_id': 'user', - 'application_status': 'REJECTED', + 'application_status': APPLICATION_REJECTED, 'remaining_credits': 0, 'job_names': [], } @@ -38,7 +39,7 @@ def test_get_user_with_jobs(client, tables): user = { 'user_id': user_id, 'remaining_credits': 20, - 'application_status': 'APPROVED', + 'application_status': APPLICATION_APPROVED, 'credits_per_month': 50, '_month_of_last_credit_reset': '2024-01-01', '_foo': 'bar', @@ -60,7 +61,7 @@ def test_get_user_with_jobs(client, tables): assert response.status_code == HTTPStatus.OK assert response.json == { 'user_id': 'user_with_jobs', - 'application_status': 'APPROVED', + 'application_status': APPLICATION_APPROVED, 'credits_per_month': 50, 'remaining_credits': 50, 'job_names': [ diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index e19592651..e28dccb97 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -13,6 +13,7 @@ RejectedApplicationError, InvalidApplicationStatusError, ) +from dynamo.user import APPLICATION_APPROVED def test_query_jobs_by_user(tables): @@ -292,7 +293,7 @@ def test_put_jobs(tables, monkeypatch, approved_user): 'user_id': approved_user, 'remaining_credits': Decimal(7), '_month_of_last_credit_reset': '2024-02', - 'application_status': 'APPROVED', + 'application_status': APPLICATION_APPROVED, }] @@ -489,7 +490,9 @@ 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', 'remaining_credits': None, 'application_status': 'APPROVED'}) + tables.users_table.put_item( + Item={'user_id': 'user1', 'remaining_credits': None, 'application_status': APPLICATION_APPROVED} + ) jobs = dynamo.jobs.put_jobs('user1', payload) @@ -500,7 +503,9 @@ def test_put_jobs_infinite_credits(tables, monkeypatch): def test_put_jobs_priority_override(tables): payload = [{'name': 'name1'}, {'name': 'name2'}] - user = {'user_id': 'user1', 'priority_override': 100, 'remaining_credits': 3, 'application_status': 'APPROVED'} + user = { + 'user_id': 'user1', 'priority_override': 100, 'remaining_credits': 3, 'application_status': APPLICATION_APPROVED + } tables.users_table.put_item(Item=user) jobs = dynamo.jobs.put_jobs('user1', payload) @@ -509,7 +514,12 @@ def test_put_jobs_priority_override(tables): for job in jobs: assert job['priority'] == 100 - user = {'user_id': 'user1', 'priority_override': 550, 'remaining_credits': None, 'application_status': 'APPROVED'} + user = { + 'user_id': 'user1', + 'priority_override': 550, + 'remaining_credits': None, + 'application_status': APPLICATION_APPROVED + } tables.users_table.put_item(Item=user) jobs = dynamo.jobs.put_jobs('user1', payload) From 4c875c24232534981d1dd86c45d3e625dd3772d2 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 12:05:39 -0800 Subject: [PATCH 64/84] remove credits_per_month when resetting unapproved user record --- lib/dynamo/dynamo/user.py | 2 +- tests/test_dynamo/test_user.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 549cb157a..061a72e63 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -128,7 +128,7 @@ def _reset_credits_if_needed(user: dict, default_credits: Decimal, current_month try: user = users_table.update_item( Key={'user_id': user['user_id']}, - UpdateExpression='SET remaining_credits = :zero REMOVE #month_of_last_credit_reset', + UpdateExpression='SET remaining_credits = :zero REMOVE #month_of_last_credit_reset, credits_per_month', ConditionExpression='application_status <> :approved AND remaining_credits <> :zero', ExpressionAttributeNames={'#month_of_last_credit_reset': '_month_of_last_credit_reset'}, ExpressionAttributeValues={':approved': APPLICATION_APPROVED, ':zero': Decimal(0)}, diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 94cc12bd8..4c18ad63a 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -369,6 +369,7 @@ def test_reset_credits_to_zero(tables): original_user_record = { 'user_id': 'foo', 'remaining_credits': Decimal(10), + 'credits_per_month': Decimal(50), '_month_of_last_credit_reset': '2024-02', 'application_status': 'bar', } From 87a53be8c0c0476028457d34bf8261e4bb95cad2 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 12:13:26 -0800 Subject: [PATCH 65/84] fix imports --- lib/dynamo/dynamo/user.py | 2 +- tests/test_api/test_get_user.py | 2 +- tests/test_api/test_patch_user.py | 3 ++- tests/test_dynamo/test_jobs.py | 2 +- tests/test_dynamo/test_user.py | 4 ++-- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index 061a72e63..b9d8867f1 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -7,7 +7,7 @@ import requests from dynamo.exceptions import ( - DatabaseConditionException, RejectedApplicationError, ApprovedApplicationError, InvalidApplicationStatusError + ApprovedApplicationError, DatabaseConditionException, InvalidApplicationStatusError, RejectedApplicationError ) from dynamo.util import DYNAMODB_RESOURCE diff --git a/tests/test_api/test_get_user.py b/tests/test_api/test_get_user.py index debd8122a..fe21ea5ed 100644 --- a/tests/test_api/test_get_user.py +++ b/tests/test_api/test_get_user.py @@ -1,9 +1,9 @@ from datetime import datetime, timezone from http import HTTPStatus -from dynamo.user import APPLICATION_APPROVED, APPLICATION_REJECTED, APPLICATION_NOT_STARTED from test_api.conftest import USER_URI, login, make_db_record +from dynamo.user import APPLICATION_APPROVED, APPLICATION_NOT_STARTED, APPLICATION_REJECTED from dynamo.util import format_time diff --git a/tests/test_api/test_patch_user.py b/tests/test_api/test_patch_user.py index 5ace5cdd5..ef3a47eef 100644 --- a/tests/test_api/test_patch_user.py +++ b/tests/test_api/test_patch_user.py @@ -2,9 +2,10 @@ from decimal import Decimal from http import HTTPStatus -from dynamo.user import APPLICATION_PENDING, APPLICATION_REJECTED from test_api.conftest import DEFAULT_ACCESS_TOKEN, USER_URI, login +from dynamo.user import APPLICATION_PENDING, APPLICATION_REJECTED + def test_patch_new_user(client, tables): login(client, 'foo') diff --git a/tests/test_dynamo/test_jobs.py b/tests/test_dynamo/test_jobs.py index e28dccb97..2b5d1b748 100644 --- a/tests/test_dynamo/test_jobs.py +++ b/tests/test_dynamo/test_jobs.py @@ -8,10 +8,10 @@ import dynamo from dynamo.exceptions import ( InsufficientCreditsError, + InvalidApplicationStatusError, NotStartedApplicationError, PendingApplicationError, RejectedApplicationError, - InvalidApplicationStatusError, ) from dynamo.user import APPLICATION_APPROVED diff --git a/tests/test_dynamo/test_user.py b/tests/test_dynamo/test_user.py index 4c18ad63a..583341119 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -6,10 +6,10 @@ import dynamo.user from dynamo.exceptions import ( - DatabaseConditionException, - RejectedApplicationError, ApprovedApplicationError, + DatabaseConditionException, InvalidApplicationStatusError, + RejectedApplicationError, ) from dynamo.user import APPLICATION_APPROVED, APPLICATION_NOT_STARTED, APPLICATION_PENDING, APPLICATION_REJECTED From a438d6c0795631cad8500e598cc93e8260731c0c Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 14:16:08 -0800 Subject: [PATCH 66/84] parameterize default application status --- lib/dynamo/dynamo/user.py | 30 ++++++++-------- tests/cfg.env | 1 + tests/test_dynamo/test_user.py | 66 ++++++++++++++++++---------------- 3 files changed, 52 insertions(+), 45 deletions(-) diff --git a/lib/dynamo/dynamo/user.py b/lib/dynamo/dynamo/user.py index b9d8867f1..bfe5fcfea 100644 --- a/lib/dynamo/dynamo/user.py +++ b/lib/dynamo/dynamo/user.py @@ -11,7 +11,7 @@ ) from dynamo.util import DYNAMODB_RESOURCE -APPLICATION_NOT_STARTED = 'NOT STARTED' +APPLICATION_NOT_STARTED = 'NOT_STARTED' APPLICATION_PENDING = 'PENDING' APPLICATION_APPROVED = 'APPROVED' APPLICATION_REJECTED = 'REJECTED' @@ -63,19 +63,15 @@ def get_or_create_user(user_id: str) -> dict: 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: - user = _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, - users_table=users_table, - ) - return user + if user is None: + user = _create_user(user_id, users_table) + + return _reset_credits_if_needed( + user=user, + default_credits=default_credits, + current_month=current_month, + users_table=users_table, + ) def _get_current_month() -> str: @@ -83,7 +79,11 @@ def _get_current_month() -> str: def _create_user(user_id: str, users_table) -> dict: - user = {'user_id': user_id, 'remaining_credits': Decimal(0), 'application_status': APPLICATION_NOT_STARTED} + user = { + 'user_id': user_id, + 'remaining_credits': Decimal(0), + 'application_status': os.environ['DEFAULT_APPLICATION_STATUS'], + } try: users_table.put_item(Item=user, ConditionExpression='attribute_not_exists(user_id)') except botocore.exceptions.ClientError as e: diff --git a/tests/cfg.env b/tests/cfg.env index a01fd7e08..31bf731a5 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 +DEFAULT_APPLICATION_STATUS=NOT_STARTED 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 583341119..ee12c0299 100644 --- a/tests/test_dynamo/test_user.py +++ b/tests/test_dynamo/test_user.py @@ -188,52 +188,58 @@ def test_update_user_failed_application_status(tables): }] -def test_get_or_create_user_reset(tables, monkeypatch): - monkeypatch.setenv('DEFAULT_CREDITS_PER_USER', '25') - tables.users_table.put_item(Item={'user_id': 'foo'}) +def test_get_or_create_user_existing_user(tables): + tables.users_table.put_item( + Item={ + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': APPLICATION_APPROVED, + } + ) - 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: + with unittest.mock.patch('dynamo.user._get_current_month') as mock_get_current_month: mock_get_current_month.return_value = '2024-02' - mock_reset_credits_if_needed.return_value = 'reset_credits_return_value' - - user = dynamo.user.get_or_create_user('foo') - + user = dynamo.user.get_or_create_user(user_id='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 == 'reset_credits_return_value' + assert user == { + 'user_id': 'foo', + 'remaining_credits': Decimal(25), + '_month_of_last_credit_reset': '2024-02', + 'application_status': APPLICATION_APPROVED, + } + assert tables.users_table.scan()['Items'] == [user] -def test_get_or_create_user_create(tables): - with unittest.mock.patch('dynamo.user._create_user') as mock_create_user: - mock_create_user.return_value = 'create_user_return_value' - user = dynamo.user.get_or_create_user('foo') +def test_get_or_create_user_new_user(tables): + user = dynamo.user.get_or_create_user(user_id='foo') - mock_create_user.assert_called_once_with( - user_id='foo', - users_table=tables.users_table, - ) + assert user == { + 'user_id': 'foo', + 'remaining_credits': Decimal(0), + 'application_status': APPLICATION_NOT_STARTED, + } + assert tables.users_table.scan()['Items'] == [user] - assert user == 'create_user_return_value' +def test_get_or_create_user_default_application_status_approved(tables, monkeypatch): + monkeypatch.setenv('DEFAULT_APPLICATION_STATUS', APPLICATION_APPROVED) -def test_create_user(tables): - user = dynamo.user._create_user(user_id='foo', users_table=tables.users_table) + 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(user_id='foo') + mock_get_current_month.assert_called_once_with() assert user == { - 'user_id': 'foo', 'remaining_credits': Decimal(0), 'application_status': APPLICATION_NOT_STARTED + 'user_id': 'foo', + 'remaining_credits': Decimal(25), + '_month_of_last_credit_reset': '2024-02', + 'application_status': APPLICATION_APPROVED, } assert tables.users_table.scan()['Items'] == [user] -def test_create_user_already_exists(tables): +def test_create_user_failed_already_exists(tables): tables.users_table.put_item(Item={'user_id': 'foo'}) with pytest.raises(DatabaseConditionException): From 0a7707cc826830f5d890022fd9fbf16cd1c7abbe Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 14:29:41 -0800 Subject: [PATCH 67/84] add default_application_status deployment param --- .github/actions/deploy-hyp3/action.yml | 4 ++++ .github/workflows/deploy-daac.yml | 3 +++ .github/workflows/deploy-enterprise-test.yml | 4 ++++ .github/workflows/deploy-enterprise.yml | 13 +++++++++++++ .github/workflows/deploy-whitelisting-sandbox.yml | 3 +++ apps/api/api-cf.yml.j2 | 4 ++++ 6 files changed, 31 insertions(+) diff --git a/.github/actions/deploy-hyp3/action.yml b/.github/actions/deploy-hyp3/action.yml index 9806764a4..52986fd3e 100644 --- a/.github/actions/deploy-hyp3/action.yml +++ b/.github/actions/deploy-hyp3/action.yml @@ -38,6 +38,9 @@ inputs: DEFAULT_CREDITS_PER_USER: description: "The default number of credits given to a new user" required: true + DEFAULT_APPLICATION_STATUS: + description: "The default status for new user applications." + required: true COST_PROFILE: description: "Job spec cost profile" required: true @@ -123,6 +126,7 @@ runs: $ORIGIN_ACCESS_IDENTITY_ID \ $DISTRIBUTION_URL \ DefaultCreditsPerUser='${{ inputs.DEFAULT_CREDITS_PER_USER }}' \ + DefaultApplicationStatus='${{ inputs.DEFAULT_APPLICATION_STATUS }}' \ DefaultMaxvCpus='${{ inputs.DEFAULT_MAX_VCPUS }}' \ ExpandedMaxvCpus='${{ inputs.EXPANDED_MAX_VCPUS }}' \ MonthlyBudget='${{ inputs.MONTHLY_BUDGET }}' \ diff --git a/.github/workflows/deploy-daac.yml b/.github/workflows/deploy-daac.yml index 667cea63f..52859d39f 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: 10000 + default_application_status: APPROVED cost_profile: EDC 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 @@ -40,6 +41,7 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 10000 + default_application_status: APPROVED cost_profile: EDC deploy_ref: refs/heads/develop job_files: >- @@ -88,6 +90,7 @@ jobs: SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} DEFAULT_CREDITS_PER_USER: ${{ matrix.default_credits_per_user }} + DEFAULT_APPLICATION_STATUS: ${{ matrix.default_application_status }} COST_PROFILE: ${{ matrix.cost_profile }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} diff --git a/.github/workflows/deploy-enterprise-test.yml b/.github/workflows/deploy-enterprise-test.yml index a5731d068..bc553b1cf 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 + default_application_status: APPROVED cost_profile: DEFAULT deploy_ref: refs/heads/develop job_files: >- @@ -43,6 +44,7 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 0 + default_application_status: APPROVED cost_profile: DEFAULT job_files: >- job_spec/ARIA_RAIDER.yml @@ -61,6 +63,7 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 0 + default_application_status: APPROVED cost_profile: DEFAULT job_files: >- job_spec/AUTORIFT_ITS_LIVE.yml @@ -106,6 +109,7 @@ jobs: SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} DEFAULT_CREDITS_PER_USER: ${{ matrix.default_credits_per_user }} + DEFAULT_APPLICATION_STATUS: ${{ matrix.default_application_status }} COST_PROFILE: ${{ matrix.cost_profile }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} diff --git a/.github/workflows/deploy-enterprise.yml b/.github/workflows/deploy-enterprise.yml index 04405e3f3..1d90caedc 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: 14 default_credits_per_user: 0 + default_application_status: APPROVED cost_profile: DEFAULT job_files: >- job_spec/AUTORIFT_ITS_LIVE.yml @@ -38,6 +39,7 @@ jobs: image_tag: latest product_lifetime_in_days: 180 default_credits_per_user: 0 + default_application_status: APPROVED cost_profile: DEFAULT job_files: >- job_spec/ARIA_RAIDER.yml @@ -56,6 +58,7 @@ jobs: image_tag: latest product_lifetime_in_days: 30 default_credits_per_user: 0 + default_application_status: APPROVED cost_profile: DEFAULT job_files: job_spec/INSAR_ISCE.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge @@ -72,6 +75,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 + default_application_status: APPROVED cost_profile: DEFAULT job_files: job_spec/INSAR_ISCE.yml instance_types: c6id.xlarge,c6id.2xlarge,c6id.4xlarge,c6id.8xlarge @@ -88,6 +92,7 @@ jobs: image_tag: latest product_lifetime_in_days: 365000 default_credits_per_user: 0 + default_application_status: APPROVED cost_profile: DEFAULT 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 @@ -104,6 +109,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 + default_application_status: APPROVED cost_profile: DEFAULT 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 @@ -120,6 +126,7 @@ jobs: image_tag: latest product_lifetime_in_days: 90 default_credits_per_user: 0 + default_application_status: APPROVED cost_profile: DEFAULT 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 @@ -136,6 +143,7 @@ jobs: image_tag: latest product_lifetime_in_days: 30 default_credits_per_user: 0 + default_application_status: APPROVED cost_profile: DEFAULT job_files: job_spec/INSAR_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 @@ -152,6 +160,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 + default_application_status: APPROVED cost_profile: DEFAULT 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 @@ -168,6 +177,7 @@ jobs: image_tag: latest product_lifetime_in_days: 14 default_credits_per_user: 0 + default_application_status: APPROVED cost_profile: DEFAULT 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 @@ -184,6 +194,7 @@ jobs: image_tag: latest product_lifetime_in_days: 30 default_credits_per_user: 0 + default_application_status: APPROVED cost_profile: DEFAULT 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 @@ -202,6 +213,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 + default_application_status: APPROVED cost_profile: DEFAULT 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 @@ -244,6 +256,7 @@ jobs: SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} DEFAULT_CREDITS_PER_USER: ${{ matrix.default_credits_per_user }} + DEFAULT_APPLICATION_STATUS: ${{ matrix.default_application_status }} COST_PROFILE: ${{ matrix.cost_profile }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} diff --git a/.github/workflows/deploy-whitelisting-sandbox.yml b/.github/workflows/deploy-whitelisting-sandbox.yml index d79478a53..6c990ee68 100644 --- a/.github/workflows/deploy-whitelisting-sandbox.yml +++ b/.github/workflows/deploy-whitelisting-sandbox.yml @@ -20,6 +20,8 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 10 + # TODO set back to NOT_STARTED + default_application_status: APPROVED cost_profile: EDC deploy_ref: refs/heads/whitelisting-sandbox job_files: >- @@ -67,6 +69,7 @@ jobs: SECRET_ARN: ${{ secrets.SECRET_ARN }} CLOUDFORMATION_ROLE_ARN: ${{ secrets.CLOUDFORMATION_ROLE_ARN }} DEFAULT_CREDITS_PER_USER: ${{ matrix.default_credits_per_user }} + DEFAULT_APPLICATION_STATUS: ${{ matrix.default_application_status }} COST_PROFILE: ${{ matrix.cost_profile }} JOB_FILES: ${{ matrix.job_files }} DEFAULT_MAX_VCPUS: ${{ matrix.default_max_vcpus }} diff --git a/apps/api/api-cf.yml.j2 b/apps/api/api-cf.yml.j2 index 2edf56f22..975bbf3be 100644 --- a/apps/api/api-cf.yml.j2 +++ b/apps/api/api-cf.yml.j2 @@ -15,6 +15,9 @@ Parameters: DefaultCreditsPerUser: Type: Number + DefaultApplicationStatus: + Type: String + SystemAvailable: Type: String @@ -179,6 +182,7 @@ Resources: AUTH_PUBLIC_KEY: !Ref AuthPublicKey AUTH_ALGORITHM: !Ref AuthAlgorithm DEFAULT_CREDITS_PER_USER: !Ref DefaultCreditsPerUser + DEFAULT_APPLICATION_STATUS: !Ref DefaultApplicationStatus SYSTEM_AVAILABLE: !Ref SystemAvailable Code: src/ Handler: hyp3_api.lambda_handler.handler From 994271645ce4e768d240c2306c35417fbf3deb72 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 14:31:00 -0800 Subject: [PATCH 68/84] update deployment docs --- docs/deployments/ASF-deployment.md | 3 ++- docs/deployments/JPL-deployment.md | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/deployments/ASF-deployment.md b/docs/deployments/ASF-deployment.md index 38c1a1489..b96e7be77 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 \ + DefaultApplicationStatus=APPROVED ``` diff --git a/docs/deployments/JPL-deployment.md b/docs/deployments/JPL-deployment.md index 45b9d25e2..830c8800f 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 \ + DefaultApplicationStatus=APPROVED ``` ## 5. Post deployment From 89147cc1b800f9ea1fba511fe42a5e39033791c2 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 14:37:26 -0800 Subject: [PATCH 69/84] add default app status to main cf --- apps/main-cf.yml.j2 | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/apps/main-cf.yml.j2 b/apps/main-cf.yml.j2 index b0fda59b8..ed7bcf976 100644 --- a/apps/main-cf.yml.j2 +++ b/apps/main-cf.yml.j2 @@ -36,6 +36,15 @@ Parameters: Type: Number MinValue: 0 + DefaultApplicationStatus: + Description: The default status for new user applications. + Type: String + AllowedValues: + - NOT_STARTED + - PENDING + - APPROVED + - REJECTED + 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 +125,7 @@ Resources: AuthPublicKey: !Ref AuthPublicKey AuthAlgorithm: !Ref AuthAlgorithm DefaultCreditsPerUser: !Ref DefaultCreditsPerUser + DefaultApplicationStatus: !Ref DefaultApplicationStatus SystemAvailable: !Ref SystemAvailable {% if security_environment == 'EDC' %} VpcId: !Ref VpcId From d5328d75c68baed0a3387cb7d1ff031f04f47d00 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 14:46:34 -0800 Subject: [PATCH 70/84] set whitelisting-sandbox default app status to NOT_STARTED --- .github/workflows/deploy-whitelisting-sandbox.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/deploy-whitelisting-sandbox.yml b/.github/workflows/deploy-whitelisting-sandbox.yml index 6c990ee68..3001fbdc1 100644 --- a/.github/workflows/deploy-whitelisting-sandbox.yml +++ b/.github/workflows/deploy-whitelisting-sandbox.yml @@ -20,8 +20,7 @@ jobs: image_tag: test product_lifetime_in_days: 14 default_credits_per_user: 10 - # TODO set back to NOT_STARTED - default_application_status: APPROVED + default_application_status: NOT_STARTED cost_profile: EDC deploy_ref: refs/heads/whitelisting-sandbox job_files: >- From 9dc62c5defc98509a8c55815280e2fc2d51784db Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 15:07:10 -0800 Subject: [PATCH 71/84] Update apps/api/src/hyp3_api/api-spec/openapi-spec.yml.j2 --- apps/api/src/hyp3_api/api-spec/openapi-spec.yml.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5cb8b6eab..98d5ea13f 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 @@ -225,7 +225,7 @@ components: description: Status of an application for processing approval. type: string enum: - - NOT STARTED + - NOT_STARTED - PENDING - APPROVED - REJECTED From faf8a2ad1321762802ef814e6d705622d17bd85d Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 15:48:16 -0800 Subject: [PATCH 72/84] changelog --- CHANGELOG.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3ce81604..143365280 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,32 @@ 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). +## [7.2.0] + +This release includes changes to support an upcoming user whitelisting feature. A new user will be required to submit an application for a monthly credit allotment and will not be able to submit HyP3 jobs until an operator has manually reviewed and approved the application. As of this release, all new and existing users are automatically approved without being required to submit an application, but this will change in the near future. + +⚠️ Important notes for HyP3 deployment operators: +- Changing a user's application status (e.g. to approve or reject a new user) requires manually updating the value of the `application_status` field in the Users table. +- The response for both `/user` endpoints now automatically includes all Users table fields except those prefixed by an underscore (`_`). +- The following manual updates must be made to the Users table upon deployment of this release: + - Add field `application_status` with the appropriate value for each user. + - Rename field `month_of_last_credits_reset` to `_month_of_last_credit_reset`. + - Rename field `notes` to `_notes`. + +### Added +- A new `PATCH /user` endpoint with a single `use_case` parameter allows the user to submit an application for a monthly credit allotment or update a pending application. The structure for a successful response is the same as for `GET /user`. +- A new `default_application_status` deployment parameter specifies the default status for new user applications. The parameter has been set to `APPROVED` for all deployments. + +### Changed +- The `POST /jobs` endpoint now returns a `403` response if the user has not been approved for a monthly credit allotment. +- The response schema for the `GET /user` endpoint now includes: + - A required `application_status` field representing the status of the user's application: `NOT_STARTED`, `PENDING`, `APPROVED`, or `REJECTED`. + - An optional `use_case` field containing the use case submitted with the user's application. + - An optional `credits_per_month` field representing the user's monthly credit allotment, if different from the deployment default. + +### Removed +- The `reset_credits_monthly` deployment parameter has been removed. Credits now reset monthly in all deployments. This only changes the behavior of the `hyp3-enterprise-test` deployment. + ## [7.1.1] ### Changed - Reduced `start_execution_manager` batch size from 600 jobs to 500 jobs. Fixes [#2241](https://github.com/ASFHyP3/hyp3/issues/2241). From 314c63cf649c83c4a1f9f39b6a9598effcfc0f1b Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Tue, 7 May 2024 16:00:11 -0800 Subject: [PATCH 73/84] remove unneeded Werkzeug pin in api requirements; see #1861 --- requirements-apps-api.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements-apps-api.txt b/requirements-apps-api.txt index ef089808a..c6b9de8ac 100644 --- a/requirements-apps-api.txt +++ b/requirements-apps-api.txt @@ -8,5 +8,4 @@ requests==2.31.0 serverless_wsgi==3.0.3 shapely==2.0.4 strict-rfc3339==0.7 -Werkzeug==3.0.2 ./lib/dynamo/ From 1d7a17fbb5c962c5e7ef364affd0d867e8f1c76e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 8 May 2024 00:05:19 +0000 Subject: [PATCH 74/84] Bump jinja2 from 3.1.3 to 3.1.4 Bumps [jinja2](https://github.com/pallets/jinja) from 3.1.3 to 3.1.4. - [Release notes](https://github.com/pallets/jinja/releases) - [Changelog](https://github.com/pallets/jinja/blob/main/CHANGES.rst) - [Commits](https://github.com/pallets/jinja/compare/3.1.3...3.1.4) --- updated-dependencies: - dependency-name: jinja2 dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- requirements-all.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-all.txt b/requirements-all.txt index 0e1de3b06..7ae1b1e8e 100644 --- a/requirements-all.txt +++ b/requirements-all.txt @@ -6,7 +6,7 @@ -r requirements-apps-disable-private-dns.txt -r requirements-apps-update-db.txt boto3==1.34.87 -jinja2==3.1.3 +jinja2==3.1.4 moto[dynamodb]==5.0.6 pytest==8.1.1 PyYAML==6.0.1 From 162e0a16ae6e556f93dd12232c88172bbf0d7a31 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 8 May 2024 00:05:28 +0000 Subject: [PATCH 75/84] Bump pytest from 8.1.1 to 8.2.0 Bumps [pytest](https://github.com/pytest-dev/pytest) from 8.1.1 to 8.2.0. - [Release notes](https://github.com/pytest-dev/pytest/releases) - [Changelog](https://github.com/pytest-dev/pytest/blob/main/CHANGELOG.rst) - [Commits](https://github.com/pytest-dev/pytest/compare/8.1.1...8.2.0) --- updated-dependencies: - dependency-name: pytest dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- requirements-all.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-all.txt b/requirements-all.txt index 0e1de3b06..74a32739a 100644 --- a/requirements-all.txt +++ b/requirements-all.txt @@ -8,7 +8,7 @@ boto3==1.34.87 jinja2==3.1.3 moto[dynamodb]==5.0.6 -pytest==8.1.1 +pytest==8.2.0 PyYAML==6.0.1 responses==0.25.0 flake8==7.0.0 From cd22ecf88042403c9e907abd75835cb1a80c647a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 8 May 2024 00:05:42 +0000 Subject: [PATCH 76/84] Bump boto3 from 1.34.87 to 1.34.100 Bumps [boto3](https://github.com/boto/boto3) from 1.34.87 to 1.34.100. - [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.87...1.34.100) --- 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 0e1de3b06..971a52f53 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.87 +boto3==1.34.100 jinja2==3.1.3 moto[dynamodb]==5.0.6 pytest==8.1.1 diff --git a/requirements-apps-disable-private-dns.txt b/requirements-apps-disable-private-dns.txt index 650714373..46c415b8b 100644 --- a/requirements-apps-disable-private-dns.txt +++ b/requirements-apps-disable-private-dns.txt @@ -1 +1 @@ -boto3==1.34.87 +boto3==1.34.100 diff --git a/requirements-apps-start-execution-manager.txt b/requirements-apps-start-execution-manager.txt index a67e4cc80..ff011fc59 100644 --- a/requirements-apps-start-execution-manager.txt +++ b/requirements-apps-start-execution-manager.txt @@ -1,3 +1,3 @@ -boto3==1.34.87 +boto3==1.34.100 ./lib/dynamo/ ./lib/lambda_logging/ diff --git a/requirements-apps-start-execution-worker.txt b/requirements-apps-start-execution-worker.txt index a553e2729..8ff916d39 100644 --- a/requirements-apps-start-execution-worker.txt +++ b/requirements-apps-start-execution-worker.txt @@ -1,2 +1,2 @@ -boto3==1.34.87 +boto3==1.34.100 ./lib/lambda_logging/ From bfede253ca9f83ba109068d6d9c9100497c3f951 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 8 May 2024 00:07:00 +0000 Subject: [PATCH 77/84] Bump flask-cors from 4.0.0 to 4.0.1 Bumps [flask-cors](https://github.com/corydolphin/flask-cors) from 4.0.0 to 4.0.1. - [Release notes](https://github.com/corydolphin/flask-cors/releases) - [Changelog](https://github.com/corydolphin/flask-cors/blob/main/CHANGELOG.md) - [Commits](https://github.com/corydolphin/flask-cors/compare/4.0.0...4.0.1) --- updated-dependencies: - dependency-name: flask-cors dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- requirements-apps-api.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-apps-api.txt b/requirements-apps-api.txt index ff4dbab56..656cfa581 100644 --- a/requirements-apps-api.txt +++ b/requirements-apps-api.txt @@ -1,5 +1,5 @@ flask==2.2.5 -Flask-Cors==4.0.0 +Flask-Cors==4.0.1 jsonschema==4.22.0 openapi-core==0.19.1 prance==23.6.21.0 From 932bd9f20f265a3ac7dcc77815ce5553074a54f1 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Tue, 7 May 2024 16:23:13 -0800 Subject: [PATCH 78/84] Update apps/main-cf.yml.j2 --- apps/main-cf.yml.j2 | 2 -- 1 file changed, 2 deletions(-) diff --git a/apps/main-cf.yml.j2 b/apps/main-cf.yml.j2 index ed7bcf976..46a17191b 100644 --- a/apps/main-cf.yml.j2 +++ b/apps/main-cf.yml.j2 @@ -41,9 +41,7 @@ Parameters: Type: String AllowedValues: - NOT_STARTED - - PENDING - APPROVED - - REJECTED SystemAvailable: Description: Set to false to shutdown system, API will run and provide errors to users, but will not accept jobs. From bd9706e24fe57452d2d38a8f556657284d2d2dfc Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Wed, 8 May 2024 14:49:10 -0800 Subject: [PATCH 79/84] revise application status error messages --- lib/dynamo/dynamo/exceptions.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/dynamo/dynamo/exceptions.py b/lib/dynamo/dynamo/exceptions.py index 5c8377ee1..dc5047ba2 100644 --- a/lib/dynamo/dynamo/exceptions.py +++ b/lib/dynamo/dynamo/exceptions.py @@ -16,32 +16,35 @@ def __init__(self, user_id: str, application_status: str): super().__init__(f'User {user_id} has an invalid application status: {application_status}') + class UnexpectedApplicationStatusError(Exception): """Raised for an unexpected user application status.""" + help_url = 'https://hyp3-docs.asf.alaska.edu/using/request_access' class NotStartedApplicationError(UnexpectedApplicationStatusError): def __init__(self, user_id: str): - # TODO replace with URL to the application form for the given deployment super().__init__( - f'User {user_id} has not yet applied for a monthly credit allotment.' - ' Please visit to submit your application.' + f'{user_id} must request access before submitting jobs. Visit {self.help_url}' ) class PendingApplicationError(UnexpectedApplicationStatusError): def __init__(self, user_id: str): - super().__init__(f'User {user_id} has a pending application, please try again later.') + super().__init__( + f"{user_id}'s request for access is being processed. For more information, visit {self.help_url}" + ) class ApprovedApplicationError(UnexpectedApplicationStatusError): def __init__(self, user_id: str): - super().__init__(f'The application for user {user_id} has already been approved.') + super().__init__( + f'{user_id} is already approved for processing. For more information, visit {self.help_url}' + ) class RejectedApplicationError(UnexpectedApplicationStatusError): def __init__(self, user_id: str): super().__init__( - f'The application for user {user_id} has been rejected.' - ' For more information, please email ASF User Services at: uso@asf.alaska.edu' + f"{user_id}'s request for access has been rejected. For more information, visit {self.help_url}" ) From 5273c2fae7e55498c85c4daf86066d528284e57a Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Wed, 8 May 2024 14:52:23 -0800 Subject: [PATCH 80/84] more message tweaks --- lib/dynamo/dynamo/exceptions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/dynamo/dynamo/exceptions.py b/lib/dynamo/dynamo/exceptions.py index dc5047ba2..ac149f2ce 100644 --- a/lib/dynamo/dynamo/exceptions.py +++ b/lib/dynamo/dynamo/exceptions.py @@ -32,14 +32,14 @@ def __init__(self, user_id: str): class PendingApplicationError(UnexpectedApplicationStatusError): def __init__(self, user_id: str): super().__init__( - f"{user_id}'s request for access is being processed. For more information, visit {self.help_url}" + f"{user_id}'s request for access is pending review. For more information, visit {self.help_url}" ) class ApprovedApplicationError(UnexpectedApplicationStatusError): def __init__(self, user_id: str): super().__init__( - f'{user_id} is already approved for processing. For more information, visit {self.help_url}' + f"{user_id}'s request for access is already approved. For more information, visit {self.help_url}" ) From 21f96ff3263c2305c7bd0774cff90fb426429e73 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Wed, 8 May 2024 14:53:04 -0800 Subject: [PATCH 81/84] fix whitespace --- lib/dynamo/dynamo/exceptions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/dynamo/dynamo/exceptions.py b/lib/dynamo/dynamo/exceptions.py index ac149f2ce..867d339f9 100644 --- a/lib/dynamo/dynamo/exceptions.py +++ b/lib/dynamo/dynamo/exceptions.py @@ -16,7 +16,6 @@ def __init__(self, user_id: str, application_status: str): super().__init__(f'User {user_id} has an invalid application status: {application_status}') - class UnexpectedApplicationStatusError(Exception): """Raised for an unexpected user application status.""" help_url = 'https://hyp3-docs.asf.alaska.edu/using/request_access' From ac9db146a94b2a46d3a70d84f84bcb49ee2112d1 Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Wed, 8 May 2024 14:59:25 -0800 Subject: [PATCH 82/84] update tests --- tests/test_api/test_patch_user.py | 2 +- tests/test_api/test_submit_job.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_api/test_patch_user.py b/tests/test_api/test_patch_user.py index ef3a47eef..90d1c8d2d 100644 --- a/tests/test_api/test_patch_user.py +++ b/tests/test_api/test_patch_user.py @@ -65,4 +65,4 @@ def test_patch_rejected_user(client, tables): response = client.patch(USER_URI, json={'use_case': 'I want data.'}) assert response.status_code == HTTPStatus.FORBIDDEN - assert 'application for user foo has been rejected' in response.json['detail'] + assert 'has been rejected' in response.json['detail'] diff --git a/tests/test_api/test_submit_job.py b/tests/test_api/test_submit_job.py index eb1272cd2..962b544f6 100644 --- a/tests/test_api/test_submit_job.py +++ b/tests/test_api/test_submit_job.py @@ -147,7 +147,7 @@ def test_submit_unapproved_user(client, tables): response = submit_batch(client, batch) assert response.status_code == HTTPStatus.FORBIDDEN - assert response.json['detail'] == 'User foo has a pending application, please try again later.' + assert 'request for access is pending review' in response.json['detail'] def test_submit_without_jobs(client): From f2d4d3eee258c0cbf27ab738daf57f91e5ffc7fc Mon Sep 17 00:00:00 2001 From: Andrew Johnston Date: Wed, 8 May 2024 15:21:47 -0800 Subject: [PATCH 83/84] update help url --- lib/dynamo/dynamo/exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dynamo/dynamo/exceptions.py b/lib/dynamo/dynamo/exceptions.py index 867d339f9..29f386ec7 100644 --- a/lib/dynamo/dynamo/exceptions.py +++ b/lib/dynamo/dynamo/exceptions.py @@ -18,7 +18,7 @@ def __init__(self, user_id: str, application_status: str): class UnexpectedApplicationStatusError(Exception): """Raised for an unexpected user application status.""" - help_url = 'https://hyp3-docs.asf.alaska.edu/using/request_access' + help_url = 'https://hyp3-docs.asf.alaska.edu/using/requesting_access' class NotStartedApplicationError(UnexpectedApplicationStatusError): From 54671ccfba24cd8c2aac726616c6157e3d56a384 Mon Sep 17 00:00:00 2001 From: Jake Herrmann Date: Thu, 9 May 2024 09:06:57 -0800 Subject: [PATCH 84/84] Improve the whitelisting Changelog entry --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 143365280..cd9467e3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [7.2.0] -This release includes changes to support an upcoming user whitelisting feature. A new user will be required to submit an application for a monthly credit allotment and will not be able to submit HyP3 jobs until an operator has manually reviewed and approved the application. As of this release, all new and existing users are automatically approved without being required to submit an application, but this will change in the near future. +This release includes changes to support an upcoming user whitelisting feature. A new user will be required to apply for HyP3 access and will not be able to submit jobs until an operator has manually reviewed and approved the application. As of this release, all new and existing users are automatically approved without being required to submit an application, but this will change in the near future. ⚠️ Important notes for HyP3 deployment operators: - Changing a user's application status (e.g. to approve or reject a new user) requires manually updating the value of the `application_status` field in the Users table. @@ -17,11 +17,11 @@ This release includes changes to support an upcoming user whitelisting feature. - Rename field `notes` to `_notes`. ### Added -- A new `PATCH /user` endpoint with a single `use_case` parameter allows the user to submit an application for a monthly credit allotment or update a pending application. The structure for a successful response is the same as for `GET /user`. +- A new `PATCH /user` endpoint with a single `use_case` parameter allows the user to submit an application or update a pending application. The structure for a successful response is the same as for `GET /user`. - A new `default_application_status` deployment parameter specifies the default status for new user applications. The parameter has been set to `APPROVED` for all deployments. ### Changed -- The `POST /jobs` endpoint now returns a `403` response if the user has not been approved for a monthly credit allotment. +- The `POST /jobs` endpoint now returns a `403` response if the user has not been approved. - The response schema for the `GET /user` endpoint now includes: - A required `application_status` field representing the status of the user's application: `NOT_STARTED`, `PENDING`, `APPROVED`, or `REJECTED`. - An optional `use_case` field containing the use case submitted with the user's application.