diff --git a/.ds.baseline b/.ds.baseline index bf8b9631a..8b6703173 100644 --- a/.ds.baseline +++ b/.ds.baseline @@ -305,7 +305,7 @@ "filename": "tests/app/service/test_rest.py", "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8", "is_verified": false, - "line_number": 1274, + "line_number": 1275, "is_secret": false } ], @@ -384,5 +384,5 @@ } ] }, - "generated_at": "2024-07-22T21:27:35Z" + "generated_at": "2024-08-01T17:38:39Z" } diff --git a/app/aws/s3.py b/app/aws/s3.py index ebdffddd5..760e4dedf 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -95,6 +95,32 @@ def get_s3_file(bucket_name, file_location, access_key, secret_key, region): return s3_file.get()["Body"].read().decode("utf-8") +def download_from_s3( + bucket_name, s3_key, local_filename, access_key, secret_key, region +): + session = Session( + aws_access_key_id=access_key, + aws_secret_access_key=secret_key, + region_name=region, + ) + s3 = session.client("s3", config=AWS_CLIENT_CONFIG) + result = None + try: + result = s3.download_file(bucket_name, s3_key, local_filename) + print(f"File downloaded successfully to {local_filename}") + except botocore.exceptions.NoCredentialsError as nce: + print("Credentials not found") + raise Exception(nce) + except botocore.exceptions.PartialCredentialsError as pce: + print("Incomplete credentials provided") + raise Exception(pce) + except Exception as e: + print(f"An error occurred {e}") + text = f"EXCEPTION {e} local_filename {local_filename}" + raise Exception(text) + return result + + def get_s3_object(bucket_name, file_location, access_key, secret_key, region): session = Session( aws_access_key_id=access_key, diff --git a/app/commands.py b/app/commands.py index 2e21c0518..789bd41ab 100644 --- a/app/commands.py +++ b/app/commands.py @@ -592,14 +592,18 @@ def process_row_from_job(job_id, job_row_number): @notify_command(name="download-csv-file-by-name") -@click.option("-f", "--csv_filename", required=True, help="csv file name") +@click.option("-f", "--csv_filename", required=True, help="S3 file location") def download_csv_file_by_name(csv_filename): - + # poetry run flask command download-csv-file-by-name -f + # cf run-task notify-api-production --command "flask command download-csv-file-by-name -f " bucket_name = current_app.config["CSV_UPLOAD_BUCKET"]["bucket"] access_key = current_app.config["CSV_UPLOAD_BUCKET"]["access_key_id"] secret = current_app.config["CSV_UPLOAD_BUCKET"]["secret_access_key"] region = current_app.config["CSV_UPLOAD_BUCKET"]["region"] - print(s3.get_s3_file(bucket_name, csv_filename, access_key, secret, region)) + + s3.download_from_s3( + bucket_name, csv_filename, "download.csv", access_key, secret, region + ) @notify_command(name="dump-sms-senders") @@ -607,7 +611,7 @@ def download_csv_file_by_name(csv_filename): def dump_sms_senders(service_name): # poetry run flask command dump-sms-senders MyServiceName - # cf run-task notify-api-production --command "flask command dump-sms-senders MyServiceName" + # cf run-task notify-api-production --command "flask command dump-sms-senders " services = get_services_by_partial_name(service_name) if len(services) > 1: raise ValueError( diff --git a/docs/all.md b/docs/all.md index 140fa6ef3..8ea57a932 100644 --- a/docs/all.md +++ b/docs/all.md @@ -1368,7 +1368,7 @@ cf run-task notify-api-production --command "flask command download-csv-file-by- locally, just do: ``` -poetry run flask command download-csv-file-by-name -f +poetry run flask command download-csv-file-by-name ``` ### Debug steps diff --git a/notifications_utils/request_helper.py b/notifications_utils/request_helper.py index eb7a2061f..1dd9a9ae1 100644 --- a/notifications_utils/request_helper.py +++ b/notifications_utils/request_helper.py @@ -84,6 +84,8 @@ class ResponseHeaderMiddleware(object): current_app.logger.error(be) elif "AttributeError" in str(be): # notify-api-1394 current_app.logger.error(be) + elif "MethodNotAllowed" in str(be): # notify-admin-1392 + current_app.logger.error(be) else: raise be diff --git a/poetry.lock b/poetry.lock index 33d0907b2..cc661a9a1 100644 --- a/poetry.lock +++ b/poetry.lock @@ -204,17 +204,17 @@ tests-no-zope = ["attrs[tests-mypy]", "cloudpickle", "hypothesis", "pympler", "p [[package]] name = "awscli" -version = "1.33.32" +version = "1.33.36" description = "Universal Command Line Environment for AWS." optional = false python-versions = ">=3.8" files = [ - {file = "awscli-1.33.32-py3-none-any.whl", hash = "sha256:94e0e348b4e98d949d01f3c9bba817ef9c0da4aabea53a5d966445618339c7dd"}, - {file = "awscli-1.33.32.tar.gz", hash = "sha256:90137622dcd7695939d64f57172901376d8ca5eb995c12e5fce50f55316c5b62"}, + {file = "awscli-1.33.36-py3-none-any.whl", hash = "sha256:b4d79946aff15f83a653d41573610c55a2f2311f7387e409074dea1d2f432037"}, + {file = "awscli-1.33.36.tar.gz", hash = "sha256:52de9dd8fb3600d8d99db7ce400d058413ce69634a476582af8e2301936f0af7"}, ] [package.dependencies] -botocore = "1.34.150" +botocore = "1.34.154" colorama = ">=0.2.5,<0.4.7" docutils = ">=0.10,<0.17" PyYAML = ">=3.10,<6.1" @@ -422,13 +422,13 @@ crt = ["botocore[crt] (>=1.21.0,<2.0a0)"] [[package]] name = "botocore" -version = "1.34.150" +version = "1.34.154" description = "Low-level, data-driven core of boto 3." optional = false python-versions = ">=3.8" files = [ - {file = "botocore-1.34.150-py3-none-any.whl", hash = "sha256:b988d47f4d502df85befce11a48002421e4e6ea4289997b5e0261bac5fa76ce6"}, - {file = "botocore-1.34.150.tar.gz", hash = "sha256:4d23387e0f076d87b637a2a35c0ff2b8daca16eace36b63ce27f65630c6b375a"}, + {file = "botocore-1.34.154-py3-none-any.whl", hash = "sha256:4eef4b1bb809b382ba9dc9c88f5fcc4a133f221a1acb693ee6bee4de9f325979"}, + {file = "botocore-1.34.154.tar.gz", hash = "sha256:64d9b4c85a504d77cb56dabb2ad717cd8e1717424a88edb458b01d1e5797262a"}, ] [package.dependencies] @@ -2818,13 +2818,13 @@ ptyprocess = ">=0.5" [[package]] name = "phonenumbers" -version = "8.13.40" +version = "8.13.42" description = "Python version of Google's common library for parsing, formatting, storing and validating international phone numbers." optional = false python-versions = "*" files = [ - {file = "phonenumbers-8.13.40-py2.py3-none-any.whl", hash = "sha256:9582752c20a1da5ec4449f7f97542bf8a793c8e2fec0ab57f767177bb8fc0b1d"}, - {file = "phonenumbers-8.13.40.tar.gz", hash = "sha256:f137c2848b8e83dd064b71881b65680584417efa202177fd330e2f7ff6c68113"}, + {file = "phonenumbers-8.13.42-py2.py3-none-any.whl", hash = "sha256:18acc22ee03116d27b26e990f53806a1770a3e05f05e1620bc09ad187f889456"}, + {file = "phonenumbers-8.13.42.tar.gz", hash = "sha256:7137904f2db3b991701e853174ce8e1cb8f540b8bfdf27617540de04c0b7bed5"}, ] [[package]] @@ -3626,17 +3626,17 @@ full = ["numpy"] [[package]] name = "redis" -version = "5.0.7" +version = "5.0.8" description = "Python client for Redis database and key-value store" optional = false python-versions = ">=3.7" files = [ - {file = "redis-5.0.7-py3-none-any.whl", hash = "sha256:0e479e24da960c690be5d9b96d21f7b918a98c0cf49af3b6fafaa0753f93a0db"}, - {file = "redis-5.0.7.tar.gz", hash = "sha256:8f611490b93c8109b50adc317b31bfd84fff31def3475b92e7e80bf39f48175b"}, + {file = "redis-5.0.8-py3-none-any.whl", hash = "sha256:56134ee08ea909106090934adc36f65c9bcbbaecea5b21ba704ba6fb561f8eb4"}, + {file = "redis-5.0.8.tar.gz", hash = "sha256:0c5b10d387568dfe0698c6fad6615750c24170e548ca2deac10c649d463e9870"}, ] [package.extras] -hiredis = ["hiredis (>=1.0.0)"] +hiredis = ["hiredis (>1.0.0)"] ocsp = ["cryptography (>=36.0.1)", "pyopenssl (==20.0.1)", "requests (>=2.26.0)"] [[package]] @@ -4739,4 +4739,4 @@ multidict = ">=4.0" [metadata] lock-version = "2.0" python-versions = "^3.12.2" -content-hash = "4ef9a352a137fe091b1084b06959b9b674c30635771abc23010d2ba560f6bfd6" +content-hash = "89b99841ebd4bd735104048c8f8f0a35bfedb700a7a5648ecabe713816e25579" diff --git a/pyproject.toml b/pyproject.toml index 3bfff266b..9f9b39ce7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,7 @@ alembic = "==1.13.2" amqp = "==5.2.0" beautifulsoup4 = "==4.12.3" boto3 = "^1.34.150" -botocore = "^1.34.150" +botocore = "^1.34.154" cachetools = "==5.4.0" celery = {version = "==5.4.0", extras = ["redis"]} certifi = ">=2022.12.7" @@ -55,7 +55,7 @@ geojson = "^3.1.0" govuk-bank-holidays = "^0.14" numpy = "^1.26.4" ordered-set = "^4.1.0" -phonenumbers = "^8.13.40" +phonenumbers = "^8.13.42" python-json-logger = "^2.0.7" pytz = "^2024.1" regex = "^2024.7.24" @@ -76,7 +76,7 @@ urllib3 = "^2.2.2" webencodings = "^0.5.1" itsdangerous = "^2.2.0" jinja2 = "^3.1.4" -redis = "^5.0.7" +redis = "^5.0.8" requests = "^2.32.3" diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 8a97046e0..0cdae7de4 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1770,9 +1770,6 @@ def test_get_all_notifications_for_service_in_order_with_post_request( assert response.status_code == 200 -@pytest.mark.skip( - reason="We can't search on recipient if recipient is not kept in the db" -) def test_get_all_notifications_for_service_filters_notifications_when_using_post_request( client, notify_db_session ): @@ -1808,7 +1805,6 @@ def test_get_all_notifications_for_service_filters_notifications_when_using_post "page": 1, "template_type": [TemplateType.SMS], "status": [NotificationStatus.CREATED, NotificationStatus.SENDING], - "to": "0855", } response = client.post( @@ -1818,7 +1814,7 @@ def test_get_all_notifications_for_service_filters_notifications_when_using_post ) resp = json.loads(response.get_data(as_text=True)) - assert len(resp["notifications"]) == 1 + assert len(resp["notifications"]) == 2 assert resp["notifications"][0]["to"] == "1" assert resp["notifications"][0]["status"] == returned_notification.status assert response.status_code == 200 diff --git a/tests/app/test_commands.py b/tests/app/test_commands.py index 7eee00bbf..46dd2b0c1 100644 --- a/tests/app/test_commands.py +++ b/tests/app/test_commands.py @@ -8,6 +8,7 @@ from app.commands import ( bulk_invite_user_to_service, create_new_service, create_test_user, + download_csv_file_by_name, fix_billable_units, insert_inbound_numbers_from_file, populate_annual_billing_with_defaults, @@ -427,8 +428,22 @@ def test_promote_user_to_platform_admin( assert user.platform_admin is True +def test_download_csv_file_by_name(notify_api, mocker): + mock_download = mocker.patch("app.commands.s3.download_from_s3") + notify_api.test_cli_runner().invoke( + download_csv_file_by_name, + [ + "-f", + "NonExistentName", + ], + ) + mock_download.assert_called_once() + + def test_promote_user_to_platform_admin_no_result_found( - notify_db_session, notify_api, sample_user + notify_db_session, + notify_api, + sample_user, ): assert sample_user.platform_admin is False