From 11a04dfc842fbae967374ea3cb825d186a51549e Mon Sep 17 00:00:00 2001 From: Janet Dewar Date: Thu, 2 Jan 2025 11:01:59 -0500 Subject: [PATCH] AN-296 Upgrade Werkzeug to 3.0.6 (#796) * Update packages * Don't upgrade greenlet * Remove special pyyaml handling * Updates for new connexion version * Additional updates * Run with uvicorn for connexion 3 * Play nice with Connexion 3 * Add univorn for non-dev * Make linter happy * Make linter happier * Update test app creation * Doc fix * Don't unnecessarily confuse us with stack traces * Update tests to not use flask_testing * Fix test * Remove unused pip constraint * Remove unused imports * Linting * Finish deleting constraints * Reorder API endpoints to make Starlette happy * Add comment warning about API endpoint ordering --- api/jobs.yaml | 167 +++++++++--------- servers/cromwell/Dockerfile | 9 +- servers/cromwell/Dockerfile.dev | 10 +- servers/cromwell/README.md | 2 +- servers/cromwell/constraints.txt | 1 - servers/cromwell/jobs/__main__.py | 20 ++- .../jobs/controllers/jobs_controller.py | 5 +- servers/cromwell/jobs/encoder.py | 6 +- servers/cromwell/jobs/test/__init__.py | 22 ++- servers/cromwell/jobs/test/test_auth_utils.py | 34 ++-- .../cromwell/jobs/test/test_job_statuses.py | 6 +- .../jobs/test/test_jobs_controller.py | 118 +++++-------- .../cromwell/jobs/test/test_task_statuses.py | 5 +- servers/cromwell/requirements.txt | 21 +-- servers/cromwell/test-requirements.txt | 1 - servers/cromwell/tox.ini | 1 - 16 files changed, 199 insertions(+), 229 deletions(-) delete mode 100644 servers/cromwell/constraints.txt diff --git a/api/jobs.yaml b/api/jobs.yaml index a1456b796..fe45e6eb6 100644 --- a/api/jobs.yaml +++ b/api/jobs.yaml @@ -16,6 +16,13 @@ enum: &TIMEFRAME - DAYS_30 - ALL_TIME +# NOTE!!! +# Due to the way the Connexion library handles this file, the order in which +# endpoints are defined is important. More narrowly-defined URL paths must be +# listed before wider ones. For example, it's important that /jobs/operationDetails +# is earlier than /jobs/{id} in the below definition. When this is done incorrectly, +# we get confusing errors about missing or incorrect parameters. + paths: '/capabilities': get: @@ -33,60 +40,56 @@ paths: tags: - Capabilities - '/jobs/{id}/abort': - post: - operationId: AbortJob - summary: Abort a job by ID + '/jobs/operationDetails': + get: + operationId: GetOperationDetails + summary: Call for operation details from the Google Pipelines API parameters: - - name: id - description: Job ID + - name: job + description: job ID required: true + in: query type: string - in: path + - name: operation + description: operation ID + type: string + in: query responses: '200': description: Success - '400': - $ref: '#/responses/BadRequest' - '401': - $ref: '#/responses/Unauthorized' + schema: + $ref: '#/definitions/JobOperationResponse' '404': $ref: '#/responses/NotFound' - '412': - $ref: '#/responses/JobTerminal' '500': $ref: '#/responses/ServerError' tags: - Jobs - '/jobs/{id}/updateLabels': - post: - operationId: UpdateJobLabels - summary: Update labels on a job. + '/jobs/tailFile': + get: + operationId: TailFileContents + summary: Get up to a certain amount (from the end) of content from the Google Storage API parameters: - - name: id - description: Job ID + - name: bucket + description: Google bucket ID required: true + in: query type: string - in: path - - name: body + - name: object + description: ID of the file stored in the Google bucket required: true - in: body - schema: - $ref: '#/definitions/UpdateJobLabelsRequest' + in: query + type: string responses: '200': description: Success schema: - $ref: '#/definitions/UpdateJobLabelsResponse' - '400': - $ref: '#/responses/BadRequest' + $ref: '#/definitions/FileContents' '404': $ref: '#/responses/NotFound' '500': $ref: '#/responses/ServerError' - '501': - description: Server does not implement this method. tags: - Jobs @@ -117,10 +120,10 @@ paths: tags: - Jobs - '/jobs/{id}': - get: - operationId: GetJob - summary: Query for job and task-level metadata for a specified job + '/jobs/{id}/abort': + post: + operationId: AbortJob + summary: Abort a job by ID parameters: - name: id description: Job ID @@ -130,75 +133,65 @@ paths: responses: '200': description: Success - schema: - $ref: '#/definitions/JobMetadataResponse' '400': $ref: '#/responses/BadRequest' '401': $ref: '#/responses/Unauthorized' '404': $ref: '#/responses/NotFound' + '412': + $ref: '#/responses/JobTerminal' '500': $ref: '#/responses/ServerError' tags: - Jobs - '/jobs/{id}/{task}/attempts': - get: - operationId: GetTaskAttempts - summary: Query for task-level metadata for a specified job + '/jobs/{id}/updateLabels': + post: + operationId: UpdateJobLabels + summary: Update labels on a job. parameters: - name: id description: Job ID required: true type: string in: path - - name: task - description: task name + - name: body required: true - type: string - in: path + in: body + schema: + $ref: '#/definitions/UpdateJobLabelsRequest' responses: '200': description: Success schema: - $ref: '#/definitions/JobAttemptsResponse' + $ref: '#/definitions/UpdateJobLabelsResponse' '400': $ref: '#/responses/BadRequest' - '401': - $ref: '#/responses/Unauthorized' '404': $ref: '#/responses/NotFound' '500': $ref: '#/responses/ServerError' + '501': + description: Server does not implement this method. tags: - Jobs - '/jobs/{id}/{task}/{index}/attempts': + '/jobs/{id}': get: - operationId: GetShardAttempts - summary: Query for shard-level metadata for a specified job + operationId: GetJob + summary: Query for job and task-level metadata for a specified job parameters: - name: id description: Job ID required: true type: string in: path - - name: task - description: task name - required: true - type: string - in: path - - name: index - description: shard index - required: true - type: string - in: path responses: '200': description: Success schema: - $ref: '#/definitions/JobAttemptsResponse' + $ref: '#/definitions/JobMetadataResponse' '400': $ref: '#/responses/BadRequest' '401': @@ -210,25 +203,30 @@ paths: tags: - Jobs - '/jobs/operationDetails': + '/jobs/{id}/{task}/attempts': get: - operationId: GetOperationDetails - summary: Call for operation details from the Google Pipelines API + operationId: GetTaskAttempts + summary: Query for task-level metadata for a specified job parameters: - - name: job - description: job ID + - name: id + description: Job ID required: true - in: query type: string - - name: operation - description: operation ID + in: path + - name: task + description: task name + required: true type: string - in: query + in: path responses: '200': description: Success schema: - $ref: '#/definitions/JobOperationResponse' + $ref: '#/definitions/JobAttemptsResponse' + '400': + $ref: '#/responses/BadRequest' + '401': + $ref: '#/responses/Unauthorized' '404': $ref: '#/responses/NotFound' '500': @@ -236,26 +234,35 @@ paths: tags: - Jobs - '/jobs/tailFile': + '/jobs/{id}/{task}/{index}/attempts': get: - operationId: TailFileContents - summary: Get up to a certain amount (from the end) of content from the Google Storage API + operationId: GetShardAttempts + summary: Query for shard-level metadata for a specified job parameters: - - name: bucket - description: Google bucket ID + - name: id + description: Job ID required: true - in: query type: string - - name: object - description: ID of the file stored in the Google bucket + in: path + - name: task + description: task name + required: true + type: string + in: path + - name: index + description: shard index required: true - in: query type: string + in: path responses: '200': description: Success schema: - $ref: '#/definitions/FileContents' + $ref: '#/definitions/JobAttemptsResponse' + '400': + $ref: '#/responses/BadRequest' + '401': + $ref: '#/responses/Unauthorized' '404': $ref: '#/responses/NotFound' '500': diff --git a/servers/cromwell/Dockerfile b/servers/cromwell/Dockerfile index 5e93df0fa..5f549e6fe 100644 --- a/servers/cromwell/Dockerfile +++ b/servers/cromwell/Dockerfile @@ -13,17 +13,10 @@ WORKDIR /app COPY --from=0 /job-manager/servers/jm_utils /app/jm_utils COPY --from=0 /job-manager/servers/cromwell/jobs /app/jobs COPY ./servers/cromwell/ /app/jobs -# Below is a link explaining where the individual PyYAML install command comes from -# https://github.com/yaml/pyyaml/issues/736#issuecomment-1653209769 -# In short, due to Cython 3 being released, PyYAML needs to have it's Cython dependency contrained, otherwise it will fail to install due to deprecated features -# However installation of PyYAML uses a "wheel", which is basically a pre-compiled version of the package -# This is problematic for requirements.txt as it cannot specify a wheel, only a source package -# So we need to install PyYAML separately with the constraint defined in constraints.txt -RUN cd jobs && PIP_CONSTRAINT=constraints.txt pip install PyYAML==5.4.1 RUN cd jobs && pip install -r requirements.txt # We installed jm_utils so don't need local copy anymore, which breaks imports RUN rm -rf jm_utils # Missing required arguments -b PORT, -e ... which must be provided by the # docker image user. -ENTRYPOINT ["gunicorn", "jobs:run()"] +ENTRYPOINT ["gunicorn", "-k uvicorn.workers.UvicornWorker", "jobs:run()"] diff --git a/servers/cromwell/Dockerfile.dev b/servers/cromwell/Dockerfile.dev index 42a19f8d2..65a68d46b 100644 --- a/servers/cromwell/Dockerfile.dev +++ b/servers/cromwell/Dockerfile.dev @@ -7,18 +7,10 @@ WORKDIR /app ADD servers/jm_utils /app/jm_utils ADD servers/cromwell/jobs /app/jobs COPY servers/cromwell/requirements.txt /app/jobs -COPY servers/cromwell/constraints.txt /app/jobs -# Below is a link explaining where the individual PyYAML install command comes from -# https://github.com/yaml/pyyaml/issues/736#issuecomment-1653209769 -# In short, due to Cython 3 being released, PyYAML needs to have it's Cython dependency contrained, otherwise it will fail to install due to deprecated features -# However installation of PyYAML uses a "wheel", which is basically a pre-compiled version of the package -# This is problematic for requirements.txt as it cannot specify a wheel, only a source package -# So we need to install PyYAML separately with the constraint defined in constraints.txt -RUN cd jobs && PIP_CONSTRAINT=constraints.txt pip install PyYAML==5.4.1 RUN cd jobs && pip install -r requirements.txt # We installed jm_utils so don't need local copy anymore, which breaks imports RUN rm -rf jm_utils # Missing required arguments -b PORT, -e ... which must be provided by the # docker image user. -ENTRYPOINT ["/bin/bash", "/scripts/await_md5_match.sh", "/app/jobs/models/.jobs.yaml.md5", "--", "gunicorn", "jobs:run()"] +ENTRYPOINT ["/bin/bash", "/scripts/await_md5_match.sh", "/app/jobs/models/.jobs.yaml.md5", "--", "gunicorn", "-k uvicorn.workers.UvicornWorker", "jobs:run()"] diff --git a/servers/cromwell/README.md b/servers/cromwell/README.md index 59589c493..c8a8612fc 100644 --- a/servers/cromwell/README.md +++ b/servers/cromwell/README.md @@ -369,5 +369,5 @@ To run unit and integration tests on the python-flask app, install [`tox`](https://github.com/tox-dev/tox). ``` cd servers/cromwell -tox -- -s +tox -- -s . ``` diff --git a/servers/cromwell/constraints.txt b/servers/cromwell/constraints.txt deleted file mode 100644 index f21f1571c..000000000 --- a/servers/cromwell/constraints.txt +++ /dev/null @@ -1 +0,0 @@ -cython<3.0.0 \ No newline at end of file diff --git a/servers/cromwell/jobs/__main__.py b/servers/cromwell/jobs/__main__.py index c69c92c03..6800ffdd7 100644 --- a/servers/cromwell/jobs/__main__.py +++ b/servers/cromwell/jobs/__main__.py @@ -7,6 +7,7 @@ from distutils.util import strtobool import connexion +from connexion.jsonifier import Jsonifier import requests from requests.auth import HTTPBasicAuth @@ -54,8 +55,12 @@ # Allow unknown args if we aren't the main program, these include flags to # gunicorn. args, _ = parser.parse_known_args() -options = {"swagger_ui": False} -app = connexion.App(__name__, specification_dir='./swagger/', options=options) + +options = connexion.options.SwaggerUIOptions(swagger_ui=False) +app = connexion.App(__name__, + specification_dir='./swagger/', + swagger_ui_options=options, + jsonifier=Jsonifier(cls=JSONEncoder)) DEFAULT_CROMWELL_CREDENTIALS = {'cromwell_user': '', 'cromwell_password': ''} # Load credentials for cromwell @@ -95,11 +100,11 @@ def loadCapabilities(capabilities_path): capabilities_config) return app.app.config['capabilities'] except IOError as io_err: - logger.exception( + logger.error( 'Failed to load capabilities config, using default display fields. %s', io_err) except TypeError as type_err: - logger.exception( + logger.error( 'Failed to load capabilities config, using default display fields. %s', type_err) @@ -109,8 +114,9 @@ def loadCapabilities(capabilities_path): app.app.config['sam_url'] = args.sam_url app.app.config['use_caas'] = args.use_caas and args.use_caas.lower() == 'true' app.app.config['include_subworkflows'] = args.include_subworkflows -app.app.json_encoder = JSONEncoder -app.add_api('swagger.yaml', base_path=args.path_prefix) +app.add_api('swagger.yaml', + base_path=args.path_prefix, + jsonifier=Jsonifier(cls=JSONEncoder)) def run(): @@ -132,4 +138,4 @@ def run(): logger.critical(err) logger.critical('Failed to connect to Cromwell: {}'.format( args.cromwell_url)) - return app.app + return app diff --git a/servers/cromwell/jobs/controllers/jobs_controller.py b/servers/cromwell/jobs/controllers/jobs_controller.py index a31e81d9a..382235c71 100644 --- a/servers/cromwell/jobs/controllers/jobs_controller.py +++ b/servers/cromwell/jobs/controllers/jobs_controller.py @@ -30,7 +30,8 @@ from jobs.models.update_job_labels_request import UpdateJobLabelsRequest from jobs.models.update_job_labels_response import UpdateJobLabelsResponse from werkzeug.exceptions import (BadRequest, Forbidden, InternalServerError, - NotFound, ServiceUnavailable, Unauthorized) + NotFound, ServiceUnavailable, Unauthorized, + UnsupportedMediaType) # This is needed to support different Python versions - this # package structure changed in Python 3.10. @@ -559,6 +560,8 @@ def handle_error(response): raise BadRequest(_get_response_message(response)) elif response.status_code == Forbidden.code: raise Forbidden(_get_response_message(response)) + elif response.status_code == UnsupportedMediaType.code: + raise UnsupportedMediaType(_get_response_message(response)) elif response.status_code == InternalServerError.code: raise InternalServerError(_get_response_message(response)) elif response.status_code == NotFound.code: diff --git a/servers/cromwell/jobs/encoder.py b/servers/cromwell/jobs/encoder.py index 21ebcdbc8..3bdd05948 100644 --- a/servers/cromwell/jobs/encoder.py +++ b/servers/cromwell/jobs/encoder.py @@ -1,9 +1,9 @@ from six import iteritems from jobs.models.base_model_ import Model -from connexion.apps.flask_app import FlaskJSONEncoder +from connexion.jsonifier import JSONEncoder as ConnexionJSONEncoder -class JSONEncoder(FlaskJSONEncoder): +class JSONEncoder(ConnexionJSONEncoder): include_nulls = False def default(self, o): @@ -16,4 +16,4 @@ def default(self, o): attr = o.attribute_map[attr] dikt[attr] = value return dikt - return FlaskJSONEncoder.default(self, o) + return ConnexionJSONEncoder.default(self, o) diff --git a/servers/cromwell/jobs/test/__init__.py b/servers/cromwell/jobs/test/__init__.py index 2e5d87f18..f8ea56e21 100644 --- a/servers/cromwell/jobs/test/__init__.py +++ b/servers/cromwell/jobs/test/__init__.py @@ -1,16 +1,22 @@ import logging import connexion -from flask_testing import TestCase +from flask import json +from connexion.jsonifier import Jsonifier from ..encoder import JSONEncoder -class BaseTestCase(TestCase): +def create_app(): + logging.getLogger('connexion.operation').setLevel('ERROR') + options = connexion.options.SwaggerUIOptions(swagger_ui=False) + app = connexion.App(__name__, + specification_dir='../swagger/', + swagger_ui_options=options, + jsonifier=Jsonifier(cls=JSONEncoder)) + app.add_api('swagger.yaml', jsonifier=Jsonifier(cls=JSONEncoder)) + return app - def create_app(self): - logging.getLogger('connexion.operation').setLevel('ERROR') - app = connexion.App(__name__, specification_dir='../swagger/') - app.app.json_encoder = JSONEncoder - app.add_api('swagger.yaml') - return app.app + +def json_dumps(o): + return json.dumps(0, cls=JSONEncoder) diff --git a/servers/cromwell/jobs/test/test_auth_utils.py b/servers/cromwell/jobs/test/test_auth_utils.py index d45eb706b..373879e2d 100644 --- a/servers/cromwell/jobs/test/test_auth_utils.py +++ b/servers/cromwell/jobs/test/test_auth_utils.py @@ -5,20 +5,24 @@ from asyncio.log import logger import requests_mock +import unittest -from ..__main__ import loadCapabilities -from ..models.capabilities_response import CapabilitiesResponse -from . import BaseTestCase +from . import create_app -class TestAuthUtils(BaseTestCase): +class TestAuthUtils(unittest.TestCase): def setUp(self): + self.app = create_app() self.base_url = 'https://test-cromwell.org' + self.client = self.app.test_client() + + def assertStatus(self, response, expectedStatus): + self.assertEqual(response.status_code, expectedStatus) @requests_mock.mock() def test_token_auth_returns_200(self, mock_request): - self.app.config.update({ + self.app.app.config.update({ 'cromwell_url': self.base_url, 'cromwell_user': '', 'cromwell_password': '', @@ -34,16 +38,14 @@ def _request_callback(request, context): query_url = self.base_url + '/query' mock_request.post(query_url, json=_request_callback) - response = self.client.open('/jobs/query', - method='POST', + response = self.client.post('/jobs/query', headers={'Authentication': 'Bearer 12345'}, - data={}, - content_type='application/json') + json="{}") self.assertStatus(response, 200) @requests_mock.mock() def test_basic_auth_returns_200(self, mock_request): - self.app.config.update({ + self.app.app.config.update({ 'cromwell_url': self.base_url, 'cromwell_user': 'user', 'cromwell_password': 'password', @@ -58,24 +60,18 @@ def _request_callback(request, context): query_url = self.base_url + '/query' mock_request.post(query_url, json=_request_callback) - response = self.client.open('/jobs/query', - method='POST', - data={}, - content_type='application/json') + response = self.client.post('/jobs/query', json="{}") self.assertStatus(response, 200) def test_no_auth_with_caas_returns_401(self): - self.app.config.update({ + self.app.app.config.update({ 'cromwell_url': self.base_url, 'cromwell_user': '', 'cromwell_password': '', 'use_caas': True, 'capabilities': {} }) - response = self.client.open('/jobs/query', - method='POST', - data={}, - content_type='application/json') + response = self.client.post('/jobs/query', data="{}") self.assertStatus(response, 401) diff --git a/servers/cromwell/jobs/test/test_job_statuses.py b/servers/cromwell/jobs/test/test_job_statuses.py index babe123b5..6e2adeedb 100644 --- a/servers/cromwell/jobs/test/test_job_statuses.py +++ b/servers/cromwell/jobs/test/test_job_statuses.py @@ -2,12 +2,12 @@ from __future__ import absolute_import -from jobs.controllers.utils import job_statuses +import unittest -from . import BaseTestCase +from jobs.controllers.utils import job_statuses -class TestJobStatuses(BaseTestCase): +class TestJobStatuses(unittest.TestCase): def test_cromwell_execution_status_converts_correctly(self): for key, status in job_statuses.ApiStatus.__dict__.items(): diff --git a/servers/cromwell/jobs/test/test_jobs_controller.py b/servers/cromwell/jobs/test/test_jobs_controller.py index d9275f14d..b4f132d3d 100644 --- a/servers/cromwell/jobs/test/test_jobs_controller.py +++ b/servers/cromwell/jobs/test/test_jobs_controller.py @@ -6,8 +6,8 @@ import dateutil.parser import requests_mock +import unittest from dateutil.tz import * -from flask import json from jobs.controllers import jobs_controller from jobs.models.extended_fields import ExtendedFields from jobs.models.query_jobs_request import QueryJobsRequest @@ -15,22 +15,27 @@ from jobs.models.update_job_labels_request import UpdateJobLabelsRequest from jobs.models.update_job_labels_response import UpdateJobLabelsResponse -from . import BaseTestCase +from . import create_app, json_dumps -class TestJobsController(BaseTestCase): +class TestJobsController(unittest.TestCase): """ JobsController integration test stubs """ maxDiff = None def setUp(self): + self.app = create_app() self.base_url = 'https://test-cromwell.org' - self.app.config.update({ + self.app.app.config.update({ 'cromwell_url': self.base_url, 'cromwell_user': 'user', 'cromwell_password': 'password', 'use_caas': False, 'capabilities': {} }) + self.client = self.app.test_client() + + def assertStatus(self, response, expectedStatus): + self.assertEqual(response.status_code, expectedStatus) @requests_mock.mock() def test_abort_job(self, mock_request): @@ -47,8 +52,7 @@ def _request_callback(request, context): abort_url = self.base_url + '/{id}/abort'.format(id=workflow_id) mock_request.post(abort_url, json=_request_callback) - response = self.client.open('/jobs/{id}/abort'.format(id=workflow_id), - method='POST') + response = self.client.post('/jobs/{id}/abort'.format(id=workflow_id)) self.assertStatus(response, 204) @requests_mock.mock() @@ -69,8 +73,7 @@ def _request_callback(request, context): abort_url = self.base_url + '/{id}/abort'.format(id=workflow_id) mock_request.post(abort_url, json=_request_callback) - response = self.client.open('/jobs/{id}/abort'.format(id=workflow_id), - method='POST') + response = self.client.post('/jobs/{id}/abort'.format(id=workflow_id)) self.assertStatus(response, 404) @requests_mock.mock() @@ -142,13 +145,11 @@ def _request_callback_get_job(request, context): payload = UpdateJobLabelsRequest( labels={"test_label": "test_label_value"}) - response = self.client.open( + response = self.client.post( '/jobs/{id}/updateLabels'.format(id=workflow_id), - method='POST', - data=json.dumps(payload), - content_type='application/json') + json=json_dumps(payload)) self.assertStatus(response, 200) - self.assertEquals(response.json, + self.assertEquals(response.json(), {"labels": { "test_label": "test_label_value" }}) @@ -224,11 +225,9 @@ def _request_callback_get_job(request, context): payload = UpdateJobLabelsRequest( labels={"new_test_label": "new_test_label_value"}) - response = self.client.open( + response = self.client.post( '/jobs/{id}/updateLabels'.format(id=workflow_id), - method='POST', - data=json.dumps(payload), - content_type='application/json') + json=json_dumps(payload)) expected_result = UpdateJobLabelsResponse.from_dict({ "labels": { @@ -238,7 +237,7 @@ def _request_callback_get_job(request, context): } }) - result = UpdateJobLabelsResponse.from_dict(response.json) + result = UpdateJobLabelsResponse.from_dict(response.json()) self.assertStatus(response, 200) self.assertDictEqual(result.labels, expected_result.labels) @@ -257,13 +256,11 @@ def _request_callback(request, context): mock_request.patch(update_label_url, json=_request_callback) payload = UpdateJobLabelsRequest(labels={"": "test_invalid_label"}) - response = self.client.open( + response = self.client.post( '/jobs/{id}/updateLabels'.format(id=workflow_id), - method='POST', - data=json.dumps(payload), - content_type='application/json') + json=json_dumps(payload)) self.assertStatus(response, 400) - self.assertEquals(json.loads(response.data)['detail'], error_message) + self.assertEquals(response.json()['detail'], error_message) @requests_mock.mock() def test_update_job_labels_internal_server_error(self, mock_request): @@ -280,13 +277,11 @@ def _request_callback(request, context): payload = UpdateJobLabelsRequest( labels={"test_label": "test_label_value"}) - response = self.client.open( + response = self.client.post( '/jobs/{id}/updateLabels'.format(id=workflow_id), - method='POST', - data=json.dumps(payload), - content_type='application/json') + json=json_dumps(payload)) self.assertStatus(response, 500) - self.assertEquals(json.loads(response.data)['detail'], error_message) + self.assertEquals(response.json()['detail'], error_message) @requests_mock.mock() def test_update_job_labels_not_found(self, mock_request): @@ -306,13 +301,11 @@ def _request_callback(request, context): payload = UpdateJobLabelsRequest( labels={"test_label": "test_label_value"}) - response = self.client.open( + response = self.client.post( '/jobs/{id}/updateLabels'.format(id=workflow_id), - method='POST', - data=json.dumps(payload), - content_type='application/json') + json=json_dumps(payload)) self.assertStatus(response, 404) - self.assertEquals(json.loads(response.data)['detail'], error_message) + self.assertEquals(response.json()['detail'], error_message) @requests_mock.mock() def test_update_job_labels_undefined_unsupported_media_type_exception( @@ -329,13 +322,11 @@ def _request_callback(request, context): mock_request.patch(update_label_url, json=_request_callback) payload = UpdateJobLabelsRequest(labels={"test_label": None}) - response = self.client.open( + response = self.client.post( '/jobs/{id}/updateLabels'.format(id=workflow_id), - headers={'Accept': 'application/json'}, - method='POST', - data=json.dumps(payload)) + json=json_dumps(payload)) self.assertStatus(response, 415) - self.assertEquals(json.loads(response.data)['detail'], error_message) + self.assertEquals(response.json()['detail'], error_message) @requests_mock.mock() def test_get_job_returns_200(self, mock_request): @@ -400,10 +391,9 @@ def _request_callback(request, context): cromwell_url = self.base_url + '/{id}/metadata'.format(id=workflow_id) mock_request.get(cromwell_url, json=_request_callback) - response = self.client.open('/jobs/{id}'.format(id=workflow_id), - method='GET') + response = self.client.get('/jobs/{id}'.format(id=workflow_id)) self.assertStatus(response, 200) - response_data = json.loads(response.data) + response_data = response.json() expected_data = { 'name': workflow_name, 'id': workflow_id, @@ -481,10 +471,9 @@ def _request_callback(request, context): cromwell_url = self.base_url + '/{id}/metadata'.format(id=workflow_id) mock_request.get(cromwell_url, json=_request_callback) - response = self.client.open('/jobs/{id}'.format(id=workflow_id), - method='GET') + response = self.client.get('/jobs/{id}'.format(id=workflow_id)) self.assertStatus(response, 200) - response_data = json.loads(response.data) + response_data = response.json() expected_data = { 'name': workflow_name, 'id': workflow_id, @@ -595,10 +584,9 @@ def _request_callback(request, context): cromwell_url = self.base_url + '/{id}/metadata'.format(id=workflow_id) mock_request.get(cromwell_url, json=_request_callback) - response = self.client.open('/jobs/{id}'.format(id=workflow_id), - method='GET') + response = self.client.get('/jobs/{id}'.format(id=workflow_id)) self.assertStatus(response, 200) - response_data = json.loads(response.data) + response_data = response.json() expected_data = { 'name': workflow_name, 'id': workflow_id, @@ -740,11 +728,10 @@ def _request_callback(request, context): cromwell_url = self.base_url + '/{id}/metadata'.format(id=workflow_id) mock_request.get(cromwell_url, json=_request_callback) - response = self.client.open('/jobs/{id}/{task}/attempts'.format( - id=workflow_id, task='task'), - method='GET') + response = self.client.get('/jobs/{id}/{task}/attempts'.format( + id=workflow_id, task='task')) self.assertStatus(response, 200) - response_data = json.loads(response.data) + response_data = response.json() expected_data = { 'attempts': [{ 'attemptNumber': 1, @@ -852,13 +839,10 @@ def _request_callback(request, context): cromwell_url = self.base_url + '/{id}/metadata'.format(id=workflow_id) mock_request.get(cromwell_url, json=_request_callback) - response = self.client.open( - '/jobs/{id}/{task}/{index}/attempts'.format(id=workflow_id, - task='task', - index=0), - method='GET') + response = self.client.get('/jobs/{id}/{task}/{index}/attempts'.format( + id=workflow_id, task='task', index=0)) self.assertStatus(response, 200) - response_data = json.loads(response.data) + response_data = response.json() expected_data = { 'attempts': [{ 'attemptNumber': 1, @@ -939,10 +923,9 @@ def _request_callback(request, context): cromwell_url = self.base_url + '/{id}/metadata'.format(id=workflow_id) mock_request.get(cromwell_url, json=_request_callback) - response = self.client.open('/jobs/{id}'.format(id=workflow_id), - method='GET') + response = self.client.get('/jobs/{id}'.format(id=workflow_id)) self.assertStatus(response, 400) - self.assertEquals(json.loads(response.data)['detail'], error_message) + self.assertEquals(response.json()['detail'], error_message) @requests_mock.mock() def test_job_not_found(self, mock_request): @@ -956,10 +939,9 @@ def _request_callback(request, context): cromwell_url = self.base_url + '/{id}/metadata'.format(id=workflow_id) mock_request.get(cromwell_url, json=_request_callback) - response = self.client.open('/jobs/{id}'.format(id=workflow_id), - method='GET') + response = self.client.get('/jobs/{id}'.format(id=workflow_id)) self.assertStatus(response, 404) - self.assertEquals(json.loads(response.data)['detail'], error_message) + self.assertEquals(response.json()['detail'], error_message) @requests_mock.mock() def test_job_internal_server_error(self, mock_request): @@ -973,10 +955,9 @@ def _request_callback(request, context): cromwell_url = self.base_url + '/{id}/metadata'.format(id=workflow_id) mock_request.get(cromwell_url, json=_request_callback) - response = self.client.open('/jobs/{id}'.format(id=workflow_id), - method='GET') + response = self.client.get('/jobs/{id}'.format(id=workflow_id)) self.assertStatus(response, 500) - self.assertEquals(json.loads(response.data)['detail'], error_message) + self.assertEquals(response.json()['detail'], error_message) @requests_mock.mock() def test_query_jobs_returns_200(self, mock_request): @@ -994,10 +975,7 @@ def _request_callback(request, context): mock_request.post(query_url, json=_request_callback) query = QueryJobsRequest() - response = self.client.open('/jobs/query', - method='POST', - data=json.dumps(query), - content_type='application/json') + response = self.client.post('/jobs/query', json=json_dumps(query)) self.assertStatus(response, 200) def test_empty_cromwell_query_params(self): diff --git a/servers/cromwell/jobs/test/test_task_statuses.py b/servers/cromwell/jobs/test/test_task_statuses.py index b5919aba1..a63dda4d2 100644 --- a/servers/cromwell/jobs/test/test_task_statuses.py +++ b/servers/cromwell/jobs/test/test_task_statuses.py @@ -3,15 +3,14 @@ from __future__ import absolute_import import itertools +import unittest from jobs.controllers import jobs_controller from jobs.controllers.utils import task_statuses from jobs.models.shard import Shard -from . import BaseTestCase - -class TestTaskStatuses(BaseTestCase): +class TestTaskStatuses(unittest.TestCase): # yapf: disable def test_cromwell_execution_to_api_maps_all_task_execution_statuses_correctly(self): self.assertEqual(task_statuses.cromwell_execution_to_api('NotStarted'), 'Submitted') diff --git a/servers/cromwell/requirements.txt b/servers/cromwell/requirements.txt index e67b3b826..d2600cf7f 100644 --- a/servers/cromwell/requirements.txt +++ b/servers/cromwell/requirements.txt @@ -2,32 +2,25 @@ certifi==2024.7.4 chardet==3.0.4 click==8.1.3 clickclick==1.2.2 -connexion==2.14.1 -Flask==2.2.5 +connexion[flask]==3.1.0 +Flask==3.0.3 gevent==23.9 greenlet==2.0.2 gunicorn==22.0.0 -idna==2.7 +idna==2.8 inflection==0.3.1 -itsdangerous==2.1.2 +itsdangerous==2.2 Jinja2==3.1.2 ../jm_utils -jsonschema==2.6.0 +jsonschema==4.17.3 MarkupSafe==2.1.1 pathlib==1.0.1 python-dateutil==2.6.0 pytz==2022.4 -# Below is a link explaining why the PyYAML package is commented out -# https://github.com/yaml/pyyaml/issues/736#issuecomment-1653209769 -# In short, due to Cython 3 being released, PyYAML needs to have it's Cython dependency contrained, otherwise it'll fail to install due to deprecated features -# However installation of PyYAML uses a "wheel", which is basically a pre-compiled version of the package -# This is problematic for requirements.txt as it cannot specify a wheel, only a source package -# So we need to install PyYAML separately with the constraint defined in constraints.txt via pip install as opposed to here -# You can see the above implemented in the Dockerfiles -# PyYAML==5.4 requests==2.28.1 six==1.11.0 swagger-spec-validator==2.7.6 typing==3.6.1 urllib3==1.26.5 -Werkzeug==2.2.3 +Werkzeug==3.0.6 +uvicorn==0.32.1 diff --git a/servers/cromwell/test-requirements.txt b/servers/cromwell/test-requirements.txt index ff27c281b..6be7455fd 100644 --- a/servers/cromwell/test-requirements.txt +++ b/servers/cromwell/test-requirements.txt @@ -1,4 +1,3 @@ -flask_testing==0.8.1 coverage>=4.0.3 nose2>=0.12.0 pluggy>=1.0.0 diff --git a/servers/cromwell/tox.ini b/servers/cromwell/tox.ini index 645784d35..fd606fa9b 100644 --- a/servers/cromwell/tox.ini +++ b/servers/cromwell/tox.ini @@ -2,7 +2,6 @@ envlist = py310 [testenv] -setenv=PIP_CONSTRAINT={toxinidir}/constraints.txt deps=-r{toxinidir}/requirements.txt -r{toxinidir}/test-requirements.txt