From e54871450e1104cad1a3320679b3887f16a6b1a8 Mon Sep 17 00:00:00 2001 From: Louis des Landes Date: Wed, 27 Feb 2019 16:22:32 +1030 Subject: [PATCH 1/8] Change postgres command to use running postgres image. Otherwise postgres server itself doesn't get started. --- cli/psql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/psql b/cli/psql index 5a24cf7..aee10ff 100755 --- a/cli/psql +++ b/cli/psql @@ -2,5 +2,5 @@ COMPOSE_ENV=${COMPOSE_ENV:-local.yml} CMD='PGPASSWORD=$POSTGRES_PASSWORD psql -h $POSTGRES_HOST -U $POSTGRES_USER -p $POSTGRES_PORT $POSTGRES_DB' -docker-compose -f $COMPOSE_ENV run --rm postgres \ +docker-compose -f $COMPOSE_ENV exec postgres \ bash -c "$CMD" From 288357ca198d6696c7e852bab3d357b1d16a9275 Mon Sep 17 00:00:00 2001 From: Louis des Landes Date: Wed, 20 Feb 2019 12:49:16 +1030 Subject: [PATCH 2/8] Add models for running tasks. Adds a TaskRun and a File model for storing results of task which have been run. Allows uploading of stdout / stderr to S3. --- .envs/.local/.django | 1 + Pipfile | 1 + Pipfile.lock | 48 ++- config/settings/base.py | 2 + config/settings/test.py | 1 + local.yml | 1 + missioncontrol/home/admin.py | 9 + .../migrations/0021_auto_20190304_0300.py | 50 +++ missioncontrol/home/models.py | 106 ++++++ missioncontrol/openapi/openapi.yaml | 303 +++++++++++++++++- missioncontrol/tests/conftest.py | 35 +- missioncontrol/tests/test_files.py | 158 +++++++++ missioncontrol/tests/test_pass_task_run.py | 35 ++ missioncontrol/v0/files.py | 49 +++ missioncontrol/v0/pass_task_run.py | 40 +++ 15 files changed, 831 insertions(+), 8 deletions(-) create mode 100644 missioncontrol/home/migrations/0021_auto_20190304_0300.py create mode 100644 missioncontrol/tests/test_files.py create mode 100644 missioncontrol/tests/test_pass_task_run.py create mode 100644 missioncontrol/v0/files.py create mode 100644 missioncontrol/v0/pass_task_run.py diff --git a/.envs/.local/.django b/.envs/.local/.django index bb2c772..c1be879 100644 --- a/.envs/.local/.django +++ b/.envs/.local/.django @@ -3,3 +3,4 @@ USE_DOCKER=yes IPYTHONDIR=/app/.ipython DJANGO_JWT_SECRET=ILIKEASCREThowlongdoesitNeedtTOBeHey +DJANGO_FILE_STORAGE_PATH=/tmp/django-file-storage \ No newline at end of file diff --git a/Pipfile b/Pipfile index e51a824..1d9cde9 100644 --- a/Pipfile +++ b/Pipfile @@ -22,6 +22,7 @@ scipy = "*" python-jose = "*" tabulate = "*" django-environ = "*" +boto3 = "*" [dev-packages] pylint = "*" diff --git a/Pipfile.lock b/Pipfile.lock index ec0a8e8..37dc100 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "ba9810e74405bae543ac50ae4e7db71da11fd57f767a6638267ac6457329ef2c" + "sha256": "c2ba47a922c9979e842128f54e3b4515b4177220eb394f28cc6febbdb696b5a2" }, "pipfile-spec": 6, "requires": { @@ -89,6 +89,21 @@ "index": "pypi", "version": "==0.8.3" }, + "boto3": { + "hashes": [ + "sha256:16e093bf505ccf004ea1cab34188af8df1df02c738a6d2f46bc42e7cbda667f8", + "sha256:6f8bf13e39f52a13a1af6eb067723d6cf28b6c09d5b72953d729bff8a88fa0b9" + ], + "index": "pypi", + "version": "==1.9.108" + }, + "botocore": { + "hashes": [ + "sha256:99f83ddd73abbf8c3484f0ac7ffde21a6fbd0bfc9f9b9afbf29ce52667737c49", + "sha256:aefb5185bd3cfd4801ed32ddd51ba6c6b7054010f907d69031f5569bebccf5be" + ], + "version": "==1.12.108" + }, "brotli": { "hashes": [ "sha256:0538dc1744fd17c314d2adc409ea7d1b779783b89fd95bcfb0c2acc93a6ea5a7", @@ -220,6 +235,14 @@ "index": "pypi", "version": "==0.4.5" }, + "docutils": { + "hashes": [ + "sha256:02aec4bd92ab067f6ff27a38a38a41173bf01bed8f89157768c1573f53e474a6", + "sha256:51e64ef2ebfb29cae1faa133b3710143496eca21c530f3f71424d77687764274", + "sha256:7a4bd47eaf6596e1295ecb11361139febe29b084a87bf005bf899f9a42edc3c6" + ], + "version": "==0.14" + }, "ecdsa": { "hashes": [ "sha256:40d002cf360d0e035cf2cb985e1308d41aaa087cbfc135b2dc2d844296ea546c", @@ -304,6 +327,13 @@ ], "version": "==2.10" }, + "jmespath": { + "hashes": [ + "sha256:3720a4b1bd659dd2eecad0666459b9788813e032b83e7ba58578e48254e0a0e6", + "sha256:bde2aef6f44302dfb30320115b17d030798de8c4110e28d5cf6cf91a7a31074c" + ], + "version": "==0.9.4" + }, "jplephem": { "hashes": [ "sha256:9dffb9f3d3f6d996ade875102431fe385e8ea422da25c8ba17b0508d9ca1282b" @@ -529,6 +559,14 @@ "index": "pypi", "version": "==3.4.8" }, + "python-dateutil": { + "hashes": [ + "sha256:7e6584c74aeed623791615e26efd690f29817a27c73085b78e4bad02493df2fb", + "sha256:c89805f6f4d64db21ed966fda138f8a5ed7a4fdbc1a8ee329ce1b74e3c74da9e" + ], + "markers": "python_version >= '2.7'", + "version": "==2.8.0" + }, "python-jose": { "hashes": [ "sha256:29701d998fe560e52f17246c3213a882a4a39da7e42c7015bcc1f7823ceaff1c", @@ -576,6 +614,13 @@ ], "version": "==4.0" }, + "s3transfer": { + "hashes": [ + "sha256:7b9ad3213bff7d357f888e0fab5101b56fa1a0548ee77d121c3a3dbfbef4cb2e", + "sha256:f23d5cb7d862b104401d9021fc82e5fa0e0cf57b7660a1331425aab0c691d021" + ], + "version": "==0.2.0" + }, "scipy": { "hashes": [ "sha256:014cb900c003b5ac81a53f2403294e8ecf37aedc315b59a6b9370dce0aa7627a", @@ -664,6 +709,7 @@ "sha256:61bf29cada3fc2fbefad4fdf059ea4bd1b4a86d2b6d15e1c7c0b582b9752fe39", "sha256:de9529817c93f27c8ccbfead6985011db27bd0ddfcdb2d86f3f663385c6a9c22" ], + "markers": "python_version >= '3.4'", "version": "==1.24.1" }, "wcwidth": { diff --git a/config/settings/base.py b/config/settings/base.py index 5dccd5f..e6d8c54 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -279,3 +279,5 @@ def immutable_file_test(path, url): WHITENOISE_IMMUTABLE_FILE_TEST = immutable_file_test + +FILE_STORAGE_PATH = env.str('DJANGO_FILE_STORAGE_PATH') \ No newline at end of file diff --git a/config/settings/test.py b/config/settings/test.py index 72d180f..b8c0482 100644 --- a/config/settings/test.py +++ b/config/settings/test.py @@ -53,3 +53,4 @@ # Your stuff... # ------------------------------------------------------------------------------ +FILE_STORAGE_PATH = 's3://bucketname/django-file-storage/' diff --git a/local.yml b/local.yml index 1e367bd..d1288f9 100644 --- a/local.yml +++ b/local.yml @@ -20,6 +20,7 @@ services: ports: - "8000:8000" command: /start + restart: always frontend: build: diff --git a/missioncontrol/home/admin.py b/missioncontrol/home/admin.py index b119b2d..40b6876 100644 --- a/missioncontrol/home/admin.py +++ b/missioncontrol/home/admin.py @@ -64,3 +64,12 @@ class TaskStackAdmin(admin.ModelAdmin): 'tasks' ) list_filter = ('environment',) + +@admin.register(models.TaskRun) +class TaskRunAdmin(admin.ModelAdmin): + list_display = ( + 'uuid', + 'task', + 'task_stack', + 'task_pass', + ) \ No newline at end of file diff --git a/missioncontrol/home/migrations/0021_auto_20190304_0300.py b/missioncontrol/home/migrations/0021_auto_20190304_0300.py new file mode 100644 index 0000000..36bdccb --- /dev/null +++ b/missioncontrol/home/migrations/0021_auto_20190304_0300.py @@ -0,0 +1,50 @@ +# Generated by Django 2.1.4 on 2019-03-04 03:00 + +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import home.models +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ('home', '0020_merge_20190124_0028'), + ] + + operations = [ + migrations.CreateModel( + name='S3File', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('cid', models.CharField(max_length=128, unique=True)), + ('what', models.TextField()), + ('where', models.TextField()), + ('path', models.TextField(blank=True, null=True)), + ('start', home.models.ISODateTimeField(default=django.utils.timezone.now, help_text='The time of the first event in the file. If instantaneous, set this and leave end as null')), + ('end', home.models.ISODateTimeField(blank=True, help_text='The time of the last event in the file. Can be blank if instantaneous file.', null=True)), + ('created', home.models.ISODateTimeField(auto_now_add=True)), + ], + bases=(models.Model, home.models.Serializable), + ), + migrations.CreateModel( + name='TaskRun', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('uuid', models.UUIDField(default=uuid.uuid4, unique=True)), + ('task', models.TextField()), + ('start_time', home.models.ISODateTimeField()), + ('end_time', home.models.ISODateTimeField()), + ('exit_code', models.IntegerField()), + ('task_pass', models.ForeignKey(db_column='pass', on_delete=django.db.models.deletion.PROTECT, to='home.Pass', to_field='uuid')), + ('task_stack', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='home.TaskStack', to_field='uuid')), + ], + bases=(models.Model, home.models.Serializable), + ), + migrations.AddField( + model_name='s3file', + name='task_run', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, related_name='files', to='home.TaskRun', to_field='uuid'), + ), + ] diff --git a/missioncontrol/home/models.py b/missioncontrol/home/models.py index 36bb8ac..cf95746 100644 --- a/missioncontrol/home/models.py +++ b/missioncontrol/home/models.py @@ -19,6 +19,7 @@ from django.utils import timezone, dateformat from pytz import UTC from skyfield.api import Topos, EarthSatellite +import boto3 from v0.time import iso, utc @@ -425,6 +426,111 @@ def __str__(self): return f"Pass: {self.uuid} - {self.satellite} - {self.groundstation} - {self.start_time}" + + +class TaskRun(models.Model, Serializable): + uuid = models.UUIDField(default=uuid4, unique=True) + task = models.TextField() + task_stack = models.ForeignKey(TaskStack, on_delete=models.PROTECT, to_field='uuid') + task_pass = models.ForeignKey(Pass, on_delete=models.PROTECT, db_column='pass', to_field='uuid') + start_time = ISODateTimeField() + end_time = ISODateTimeField() + exit_code = models.IntegerField() + + def to_dict(self): + # Because connexion expects the 'pass' key, but it's a keyword + retval = super().to_dict() + retval['pass'] = retval.pop('task_pass') + # TODO possibly generalize this higher using a property on `Meta` + retval['stdout'] = self.stdout + retval['stderr'] = self.stderr + + return retval + + def _get_file_cid_if_exists(self, what): + try: + f = self.files.get(what=what) + except S3File.DoesNotExist: + return None + return f.cid + + @property + def stdout(self): + # return [x.to_dict() for x in self.files.all()] + return self._get_file_cid_if_exists('stdout') + + @property + def stderr(self): + return self._get_file_cid_if_exists('stderr') + + def __repr__(self): + print(self.task_pass) + return "".format(**self.__dict__) + + def __str__(self): + return self.__repr__() + + + +class S3File(models.Model, Serializable): + cid = models.CharField(max_length=128, unique=True) + what = models.TextField() + where = models.TextField() + path = models.TextField(null=True, blank=True) + start = ISODateTimeField( + help_text='The time of the first event in the file. ' + 'If instantaneous, set this and leave end as null', + default=timezone.now) + end = ISODateTimeField( + help_text='The time of the last event in the file. ' + 'Can be blank if instantaneous file.', + null=True, blank=True) + created = ISODateTimeField(auto_now_add=True) + task_run = models.ForeignKey(TaskRun, to_field='uuid', related_name='files', + on_delete=models.PROTECT, null=True, blank=True) + + # s3://bucket/some_path + @property + def prefix(self): + path = settings.FILE_STORAGE_PATH + if path.startswith('s3://'): + return '/'.join(path.split('/')[3:]) + raise NotImplementedError("Not yet implemented non s3 paths") + + @property + def bucket(self): + path = settings.FILE_STORAGE_PATH + if path.startswith('s3://'): + return path.split('/')[2] + raise NotImplementedError("Not yet implemented non s3 paths") + + @property + def key(self): + return f'{self.prefix}{self.cid}' + + def get_download_url(self): + s3 = boto3.client('s3') + url = s3.generate_presigned_url( + ClientMethod='get_object', + Params={ + 'Bucket': self.bucket, + 'Key': self.key, + } + ) + + return url + + def get_upload_url(self): + s3 = boto3.client('s3') + post = s3.generate_presigned_post( + Bucket=self.bucket, + Key=self.key, + ) + + return post + + + class CachedAccess(models.Model): # we store computed accesses by bucket_hash, where the hash is # hash(tle1, tle2, lat, lng, el, horizon_mask) + bucket_start + bucket_end diff --git a/missioncontrol/openapi/openapi.yaml b/missioncontrol/openapi/openapi.yaml index 67a7992..67cb117 100644 --- a/missioncontrol/openapi/openapi.yaml +++ b/missioncontrol/openapi/openapi.yaml @@ -867,6 +867,102 @@ paths: schema: $ref: '#/components/schemas/Error' + /passes/{pass_uuid}/task-runs/: + get: + tags: ['passes', 'task-runs'] + description: | + Get a list of task runs for this pass + operationId: v0.pass_task_run.search + parameters: + - in: path + required: true + name: pass_uuid + schema: + type: string + format: uuid + responses: + 200: + description: A list of task runs + content: + application/json: + schema: + $ref: "#/components/schemas/TaskRuns" + default: + description: unexpected error + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + + /passes/{pass_uuid}/task-runs/{uuid}/: + get: + tags: ['passes', 'task-runs'] + description: Get a task run from this pass + operationId: v0.pass_task_run.get + parameters: + - in: path + required: true + name: pass_uuid + schema: + type: string + format: uuid + - in: path + required: true + name: uuid + schema: + type: string + format: uuid + responses: + 200: + description: A task run + content: + application/json: + schema: + $ref: "#/components/schemas/TaskRun" + default: + description: unexpected error + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + put: + tags: ['passes', 'task-runs'] + description: Put a new task run + operationId: v0.pass_task_run.put + parameters: + - in: path + required: true + name: pass_uuid + schema: + type: string + format: uuid + - in: path + required: true + name: uuid + schema: + type: string + format: uuid + requestBody: + content: + application/json: + schema: + x-body-name: task_run + $ref: "#/components/schemas/TaskRun" + responses: + 200: + description: A task run + content: + application/json: + schema: + $ref: "#/components/schemas/TaskRun" + default: + description: unexpected error + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + + /accesses/: get: tags: ['accesses'] @@ -999,6 +1095,148 @@ paths: application/json: schema: $ref: '#/components/schemas/Error' + # Some of the structure here taken from + # https://github.com/planetlabs/datalake-api + # and modified to suit. + # Copyright 2016 Planet Labs Inc. + /files/: + get: + tags: ['files'] + description: Search for files + operationId: v0.files.search + parameters: + - in: query + name: cid + description: + The content ID of a file (blake2b hash) + schema: + type: string + - in: query + name: what + description: + Only return files from here. + schema: + type: string + required: true + - in: query + name: where + description: + Only return files from here. + schema: + type: string + - in: query + name: task_run + description: + Only return files with this task_run uuid. + schema: + type: string + format: uuid + - in: query + name: start + description: + Only return files with data after (inclusive) this start time. + schema: + type: string + format: date-time + - in: query + name: end + description: + Only return files with data before (inclusive) this end time. + schema: + type: string + format: date-time + - in: query + name: path + description: + Only return files with this path name + schema: + type: string + responses: + 200: + description: A list of files + content: + application/json: + schema: + $ref: '#/components/schemas/Files' + default: + description: unexpected error + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + /files/{cid}/: + get: + tags: ['files'] + description: Get the details of a file + operationId: v0.files.get + parameters: + - in: path + required: true + name: cid + schema: + type: string + responses: + 200: + description: The details of a file + content: + application/json: + schema: + $ref: '#/components/schemas/File' + default: + description: unexpected error + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + put: + tags: ['files'] + description: Create a new file container + operationId: v0.files.put + parameters: + - in: path + required: true + name: cid + schema: + type: string + requestBody: + content: + application/json: + schema: + x-body-name: file_body + $ref: "#/components/schemas/File" + responses: + 200: + description: The details of a file + content: + application/json: + schema: + $ref: '#/components/schemas/File' + default: + description: unexpected error + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + /files/{cid}/data/: + get: + tags: ['files'] + description: Get the data contents of a file + operationId: v0.files.get_data + parameters: + - in: path + required: true + name: cid + schema: + type: string + responses: + 302: + description: A redirect to the URL to get the data from + default: + description: unexpected error + content: + application/json: + schema: + $ref: '#/components/schemas/Error' components: securitySchemes: jwt: @@ -1223,6 +1461,11 @@ components: readOnly: true type: string + TaskRuns: + type: array + items: + "$ref": "#/components/schemas/TaskRun" + TaskRun: properties: uuid: @@ -1235,7 +1478,7 @@ components: task_stack: description: The task stack that spawned this task type: string - format: uri + format: uuid pass: description: A link to the pass that this task was run on type: string @@ -1252,17 +1495,15 @@ components: description: the exit code of the process type: integer stdout: - description: a link to the stdout log + description: a cid of the stdout log file readOnly: true type: string - format: uri stderr: - description: a link to the stderr log + description: a cid of the stderr log file readOnly: true type: string - format: uri _href: - description: link to this TaskStack + description: link to this TaskRun readOnly: true type: string format: uri @@ -1272,6 +1513,56 @@ components: additionalProperties: type: string + Files: + type: array + items: + "$ref": "#/components/schemas/File" + + File: + required: + - cid + - what + - where + - start + properties: + cid: + type: string + what: + type: string + where: + type: string + created: + type: string + format: date-time + readOnly: true + start: + type: string + format: date-time + end: + type: string + format: date-time + nullable: true + task_run: + type: string + format: uuid + nullable: true + post_url_fields: + "$ref": "#/components/schemas/PostUrlFields" + readOnly: true + + + PostUrlFields: + properties: + url: + description: The url to post to + type: string + format: uri + readOnly: true + url_fields: + description: Fields to add to the post request + type: object + readOnly: true + Error: description: schema for problem+json (RFC 7807) type: object diff --git a/missioncontrol/tests/conftest.py b/missioncontrol/tests/conftest.py index 3b91ec0..30425be 100644 --- a/missioncontrol/tests/conftest.py +++ b/missioncontrol/tests/conftest.py @@ -1,9 +1,12 @@ -import pytest +import hashlib from base64 import b64encode from uuid import uuid4 +import pytest + from flask.testing import FlaskClient from django.contrib.auth.models import User +from django.utils import timezone class AuthorizedClient(FlaskClient): def __init__(self, *args, **kwargs): @@ -103,12 +106,34 @@ def simple_sat(): @pytest.fixture def simple_pass(simple_sat, simple_gs): return { + "uuid": "9f6236cc-6bce-4e78-b8fa-8de758c20d73", "satellite": simple_sat["hwid"], "groundstation": simple_gs["hwid"], "start_time": "2018-11-25T00:00:00.000000Z", "end_time": "2018-11-25T01:00:00.000000Z", } +@pytest.fixture +def simple_task_run(simple_pass, simple_task_stack): + return { + "start_time": "2018-11-25T00:00:00.000000Z", + "end_time": "2018-11-25T01:00:00.000000Z", + "exit_code": -1, + "task": "A task name", + "task_stack": simple_task_stack["uuid"], + } + +@pytest.fixture +def simple_file(some_hash): + return { + 'cid': some_hash, + 'what': 'stdout', + 'start': "2018-11-25T01:00:00.000000Z", + 'end': None, + 'task_run': None, + 'path': '/some/path/for/files', + 'where': 'somewhere hidden', + } @pytest.fixture def some_uuid(): @@ -118,3 +143,11 @@ def some_uuid(): @pytest.fixture def another_uuid(): return str(uuid4()) + +@pytest.fixture +def yet_another_uuid(): + return str(uuid4()) + +@pytest.fixture +def some_hash(): + return hashlib.blake2b(uuid4().bytes).hexdigest() \ No newline at end of file diff --git a/missioncontrol/tests/test_files.py b/missioncontrol/tests/test_files.py new file mode 100644 index 0000000..ccdb8c4 --- /dev/null +++ b/missioncontrol/tests/test_files.py @@ -0,0 +1,158 @@ +import uuid +from unittest.mock import patch, call + +import pytest +from django.conf import settings +from django.utils import timezone, dateformat + + +@patch('home.models.boto3') +@pytest.mark.django_db +def test_file_put_get(boto3_mock, test_client, some_hash, simple_file): + post_values = { + 'url': 'https://test.example', + 'url_fields': {}, + } + boto3_mock.client.return_value.generate_presigned_post.return_value = post_values + + created = timezone.now() + with patch('django.utils.timezone.now', return_value=created): + response = test_client.put(f'/api/v0/files/{some_hash}/', json=simple_file) + assert response.status_code == 201, response.get_data() + response = test_client.get(f'/api/v0/files/{some_hash}/') + + assert response.status_code == 200, response.get_data() + expected = simple_file.copy() + expected['post_url_fields'] = post_values + + + formatter = dateformat.DateFormat(created) + expected['created'] = formatter.format(settings.DATETIME_FORMAT) + assert response.json == expected + + try: + boto3_mock.assert_has_calls([ + call.client('s3'), + call.client().generate_presigned_post( + Bucket='bucketname', Key=f'django-file-storage/{some_hash}'), + call.client('s3'), + call.client().generate_presigned_post( + Bucket='bucketname', Key=f'django-file-storage/{some_hash}'), + ], + any_order=True + ) is None + except AssertionError: + assert boto3_mock.mock_calls == [] + + + + +@patch('home.models.boto3') +@pytest.mark.django_db +def test_file_download(boto3_mock, test_client, simple_file, some_hash): + test_url = 'http://someurl' + boto3_mock.client.return_value.generate_presigned_url.return_value = test_url + response = test_client.put(f'/api/v0/files/{some_hash}/', json=simple_file) + assert response.status_code == 201, response.get_data() + + response = test_client.get(f'/api/v0/files/{some_hash}/data/') + assert response.status_code == 302, response.get_data() + + assert response.headers['Location'] == test_url + + +@pytest.mark.django_db +def test_file_search_empty(test_client, some_uuid): + response = test_client.get( + f'/api/v0/files/', + query_string={ + 'what': 'a thing', + 'cid': 'nope', + 'where': 'nowhere', + 'task_run': some_uuid, + 'start': "2018-11-25T00:00:00.000000Z", + 'end': "2018-11-25T00:00:00.000000Z", + }) + assert response.status_code == 200, response.get_data() + + assert response.json == [] + +@pytest.mark.django_db +def test_file_search_one_key(test_client): + response = test_client.get( + f'/api/v0/files/', + query_string={ + 'what': 'a thing', + }) + assert response.status_code == 200, response.get_data() + + assert response.json == [] + +@pytest.mark.django_db +def test_file_search_required_what(test_client): + response = test_client.get( + f'/api/v0/files/', + query_string={ + 'where': 'a thing', + }) + assert response.status_code == 400, response.get_data() + + assert response.json['detail'] == "Missing query parameter 'what'" + +@patch('home.models.boto3') +@pytest.mark.django_db +def test_file_search_after_put(boto_patch, test_client, simple_task_run, + simple_pass, another_uuid, some_uuid, some_hash, + simple_file, simple_sat, simple_gs, + yet_another_uuid, simple_task_stack): + def create_asset(asset_type, asset): + asset_hwid = asset.get("hwid", None) or asset.get("uuid") + response = test_client.put( + f"/api/v0/{asset_type}s/{asset_hwid}/", json=asset) + + create_asset('satellite', simple_sat) + create_asset('groundstation', simple_gs) + create_asset('task-stack', simple_task_stack) + create_asset('passe', simple_pass) + + response = test_client.put( + f'/api/v0/passes/{simple_pass["uuid"]}/task-runs/{some_uuid}/', + json=simple_task_run) + assert response.status_code == 201, response.get_data() + + simple_file['task_run'] = some_uuid + created = timezone.now() + with patch('django.utils.timezone.now', return_value=created): + response = test_client.put(f'/api/v0/files/{some_hash}/', json=simple_file) + assert response.status_code == 201, response.get_data() + + expected = simple_file.copy() + formatter = dateformat.DateFormat(created) + expected['created'] = formatter.format(settings.DATETIME_FORMAT) + + simple_file['end'] = simple_file['start'] + + response = test_client.get(f'/api/v0/files/', query_string=simple_file) + assert response.status_code == 200, response.get_data() + + assert response.json == [expected] + + # Make sure changing some of the searches *DON'T* find it. + simple_query_false = simple_file.copy() + simple_query_false['task_run'] = uuid.uuid4() + response = test_client.get(f'/api/v0/files/', query_string=simple_query_false) + assert response.status_code == 200, response.get_data() + assert response.json == [] + + # Check that querying the task_run gets it + with patch('home.models.S3File.get_download_url') as obj: + obj.return_value = 'aURLhere' + response = test_client.get(f'/api/v0/passes/{simple_pass["uuid"]}/task-runs/{some_uuid}/') + assert response.status_code == 200, response.get_data() + expected = simple_task_run.copy() + expected['pass'] = simple_pass['uuid'] + expected['uuid'] = some_uuid + expected['stdout'] = simple_file['cid'] + expected['stderr'] = None + + assert response.json == expected \ No newline at end of file diff --git a/missioncontrol/tests/test_pass_task_run.py b/missioncontrol/tests/test_pass_task_run.py new file mode 100644 index 0000000..3209de3 --- /dev/null +++ b/missioncontrol/tests/test_pass_task_run.py @@ -0,0 +1,35 @@ +import json +import pytest + + +@pytest.mark.django_db +def test_pass_task_run(test_client, simple_task_stack, simple_pass, + simple_sat, simple_gs, simple_task_run, some_uuid, another_uuid, yet_another_uuid): + + def create_asset(asset_type, asset): + asset_hwid = asset["hwid"] + response = test_client.put( + f"/api/v0/{asset_type}s/{asset_hwid}/", json=asset) + + create_asset('satellite', simple_sat) + create_asset('groundstation', simple_gs) + response = test_client.put(f'/api/v0/passes/{some_uuid}/', json=simple_pass) + assert response.status_code == 201, f"status code {response.status_code} not 201. Data: {response.get_data()}" + response = test_client.put(f'/api/v0/task-stacks/{yet_another_uuid}/', json=simple_task_stack) + assert response.status_code == 201, f"status code {response.status_code} not 201. Data: {response.get_data()}" + + # TODO test this not being used + simple_pass['task_stack'] = yet_another_uuid + simple_task_run['task_stack'] = yet_another_uuid + + # Create a task_run + response = test_client.put(f"/api/v0/passes/{some_uuid}/task-runs/{another_uuid}/", json=simple_task_run) + + expected = simple_task_run.copy() + expected['pass'] = some_uuid + expected['uuid'] = another_uuid + expected['stdout'] = None + expected['stderr'] = None + + assert response.status_code == 201, f"status code {response.status_code} not 201. Data: {response.get_data()}" + assert response.json == expected diff --git a/missioncontrol/v0/files.py b/missioncontrol/v0/files.py new file mode 100644 index 0000000..b507a57 --- /dev/null +++ b/missioncontrol/v0/files.py @@ -0,0 +1,49 @@ +import requests + +from django.conf import settings +from django.db.models import Q + +from home.models import S3File, TaskRun + +def search(**kwargs): + # remove some unused ones + kwargs.pop('token_info', None) + kwargs.pop('user', None) + + start = kwargs.pop('start', None) + # Some data + args = [] + if start: + args.append(Q(end__gte=start) | (Q(end__isnull=True) & Q(start__gte=start))) + end = kwargs.pop('end', None) + if end: + kwargs['start__lte'] = end + results = S3File.objects.filter(*args, **kwargs) + return [x.to_dict() for x in results] + + +def get_data(cid): + obj = S3File.objects.get(cid=cid) + url = obj.get_download_url() + headers = {'Location': url} + return '', 302, headers + +def get(cid): + obj = S3File.objects.get(cid=cid) + retval = obj.to_dict() + retval['post_url_fields'] = obj.get_upload_url() + return retval + +def put(cid, file_body): + task_run_uuid = file_body.pop('task_run', None) + task_run = None + if task_run_uuid: + # See if it exists + task_run = TaskRun.objects.get(uuid=task_run_uuid) + file_body['task_run'] = task_run + + obj, created = S3File.objects.update_or_create(cid=cid, defaults=file_body) + + retval = obj.to_dict() + retval['post_url_fields'] = obj.get_upload_url() + return retval, 201 if created else 200 \ No newline at end of file diff --git a/missioncontrol/v0/pass_task_run.py b/missioncontrol/v0/pass_task_run.py new file mode 100644 index 0000000..fcbb9c7 --- /dev/null +++ b/missioncontrol/v0/pass_task_run.py @@ -0,0 +1,40 @@ +from connexion.exceptions import ProblemException + +from home.models import TaskRun, Pass, TaskStack + + +def search(pass_uuid=None): + return [x.to_dict() for x in TaskRun.objects.all()] + +def get(pass_uuid=None, uuid=None): + result = TaskRun.objects.get(uuid=uuid, task_pass=pass_uuid) + return result.to_dict() + +def put(pass_uuid=None, uuid=None, task_run=None): + _pass = Pass.objects.get(uuid=pass_uuid) + task_stack = TaskStack.objects.get(uuid=task_run['task_stack']) + + + pass_uuid_body = task_run.pop('pass', None) + if pass_uuid_body and pass_uuid_body != pass_uuid: + raise ProblemException( + status=409, + title='Conflict', + detail='pass in url does not match body', + ) + + task_run["uuid"] = uuid + task_run["task_pass"] = _pass + task_run["task_stack"] = task_stack + + tr_obj, created = TaskRun.objects.get_or_create(uuid=uuid, defaults=task_run) + + result = tr_obj.to_dict() + if not created: + raise ProblemException( + status=409, + title='Conflict', + detail='The provided task run already exists', + ext={'task_run': result} + ) + return result, 201 From 926cb075312fb55fe159d6b59839068a82b9695b Mon Sep 17 00:00:00 2001 From: Louis des Landes Date: Wed, 27 Feb 2019 17:58:40 +1030 Subject: [PATCH 3/8] Add todo to use Stubber for mocks --- missioncontrol/tests/test_files.py | 1 + 1 file changed, 1 insertion(+) diff --git a/missioncontrol/tests/test_files.py b/missioncontrol/tests/test_files.py index ccdb8c4..cdbea94 100644 --- a/missioncontrol/tests/test_files.py +++ b/missioncontrol/tests/test_files.py @@ -6,6 +6,7 @@ from django.utils import timezone, dateformat +# TODO use botocore.Stubber @patch('home.models.boto3') @pytest.mark.django_db def test_file_put_get(boto3_mock, test_client, some_hash, simple_file): From 78a91905a787f7ebdee8f07ebbd7d44b0d7ad5fa Mon Sep 17 00:00:00 2001 From: Louis des Landes Date: Mon, 4 Mar 2019 16:01:36 +1030 Subject: [PATCH 4/8] Fix task-run search returning all task-runs. --- missioncontrol/tests/conftest.py | 10 ++++++ missioncontrol/tests/test_pass_task_run.py | 38 ++++++++++++++++++++++ missioncontrol/v0/pass_task_run.py | 2 +- 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/missioncontrol/tests/conftest.py b/missioncontrol/tests/conftest.py index 30425be..1115c9e 100644 --- a/missioncontrol/tests/conftest.py +++ b/missioncontrol/tests/conftest.py @@ -113,6 +113,16 @@ def simple_pass(simple_sat, simple_gs): "end_time": "2018-11-25T01:00:00.000000Z", } +@pytest.fixture +def simple_pass2(simple_sat, simple_gs): + return { + "uuid": "9f6236cc-6bce-4e78-b8fa-8de758c20d74", + "satellite": simple_sat["hwid"], + "groundstation": simple_gs["hwid"], + "start_time": "2019-11-25T00:00:00.000000Z", + "end_time": "2019-11-25T01:00:00.000000Z", + } + @pytest.fixture def simple_task_run(simple_pass, simple_task_stack): return { diff --git a/missioncontrol/tests/test_pass_task_run.py b/missioncontrol/tests/test_pass_task_run.py index 3209de3..406c4de 100644 --- a/missioncontrol/tests/test_pass_task_run.py +++ b/missioncontrol/tests/test_pass_task_run.py @@ -1,4 +1,6 @@ import json +from uuid import uuid4 + import pytest @@ -33,3 +35,39 @@ def create_asset(asset_type, asset): assert response.status_code == 201, f"status code {response.status_code} not 201. Data: {response.get_data()}" assert response.json == expected + +@pytest.mark.django_db +def test_pass_task_run_search(test_client, simple_task_stack, simple_pass, simple_pass2, + simple_sat, simple_gs, simple_task_run, some_uuid, another_uuid, yet_another_uuid): + def create_asset(asset_type, asset): + asset_hwid = asset["hwid"] + response = test_client.put( + f"/api/v0/{asset_type}s/{asset_hwid}/", json=asset) + + diff_uuid = uuid4() + + create_asset('satellite', simple_sat) + create_asset('groundstation', simple_gs) + response = test_client.put(f'/api/v0/passes/{some_uuid}/', json=simple_pass) + assert response.status_code == 201, f"status code {response.status_code} not 201. Data: {response.get_data()}" + + response = test_client.put(f'/api/v0/passes/{diff_uuid}/', json=simple_pass2) + assert response.status_code == 201, f"status code {response.status_code} not 201. Data: {response.get_data()}" + + + response = test_client.put(f'/api/v0/task-stacks/{yet_another_uuid}/', json=simple_task_stack) + assert response.status_code == 201, f"status code {response.status_code} not 201. Data: {response.get_data()}" + + simple_task_run['task_stack'] = yet_another_uuid + # Create a task_run + response = test_client.put(f"/api/v0/passes/{some_uuid}/task-runs/{another_uuid}/", json=simple_task_run) + assert response.status_code == 201, response.get_data() + + response = test_client.get(f'/api/v0/passes/{some_uuid}/task-runs/') + assert response.status_code == 200, response.get_data() + assert response.json != [] + + response = test_client.get(f'/api/v0/passes/{diff_uuid}/task-runs/') + assert response.status_code == 200, response.get_data() + assert response.json == [] + diff --git a/missioncontrol/v0/pass_task_run.py b/missioncontrol/v0/pass_task_run.py index fcbb9c7..3f0d82f 100644 --- a/missioncontrol/v0/pass_task_run.py +++ b/missioncontrol/v0/pass_task_run.py @@ -4,7 +4,7 @@ def search(pass_uuid=None): - return [x.to_dict() for x in TaskRun.objects.all()] + return [x.to_dict() for x in TaskRun.objects.filter(task_pass=pass_uuid)] def get(pass_uuid=None, uuid=None): result = TaskRun.objects.get(uuid=uuid, task_pass=pass_uuid) From 5ace1ddbade950193487ee24d1168dfde5160ef0 Mon Sep 17 00:00:00 2001 From: Louis des Landes Date: Tue, 5 Mar 2019 10:09:34 +1030 Subject: [PATCH 5/8] change start / end on task runs to range_start / range_end To better match the rest of the API. --- missioncontrol/openapi/openapi.yaml | 4 ++-- missioncontrol/tests/test_files.py | 11 +++++++---- missioncontrol/v0/files.py | 4 ++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/missioncontrol/openapi/openapi.yaml b/missioncontrol/openapi/openapi.yaml index 67cb117..5eab3cd 100644 --- a/missioncontrol/openapi/openapi.yaml +++ b/missioncontrol/openapi/openapi.yaml @@ -1132,14 +1132,14 @@ paths: type: string format: uuid - in: query - name: start + name: range_start description: Only return files with data after (inclusive) this start time. schema: type: string format: date-time - in: query - name: end + name: range_end description: Only return files with data before (inclusive) this end time. schema: diff --git a/missioncontrol/tests/test_files.py b/missioncontrol/tests/test_files.py index cdbea94..cab17ff 100644 --- a/missioncontrol/tests/test_files.py +++ b/missioncontrol/tests/test_files.py @@ -71,8 +71,8 @@ def test_file_search_empty(test_client, some_uuid): 'cid': 'nope', 'where': 'nowhere', 'task_run': some_uuid, - 'start': "2018-11-25T00:00:00.000000Z", - 'end': "2018-11-25T00:00:00.000000Z", + 'range_start': "2018-11-25T00:00:00.000000Z", + 'range_end': "2018-11-25T00:00:00.000000Z", }) assert response.status_code == 200, response.get_data() @@ -131,9 +131,11 @@ def create_asset(asset_type, asset): formatter = dateformat.DateFormat(created) expected['created'] = formatter.format(settings.DATETIME_FORMAT) - simple_file['end'] = simple_file['start'] + search_query = simple_file.copy() + search_query['range_start'] = search_query.pop('start') + search_query['range_end'] = search_query['range_start'] - response = test_client.get(f'/api/v0/files/', query_string=simple_file) + response = test_client.get(f'/api/v0/files/', query_string=search_query) assert response.status_code == 200, response.get_data() assert response.json == [expected] @@ -141,6 +143,7 @@ def create_asset(asset_type, asset): # Make sure changing some of the searches *DON'T* find it. simple_query_false = simple_file.copy() simple_query_false['task_run'] = uuid.uuid4() + simple_query_false['range_start'] = simple_query_false.pop('start') response = test_client.get(f'/api/v0/files/', query_string=simple_query_false) assert response.status_code == 200, response.get_data() assert response.json == [] diff --git a/missioncontrol/v0/files.py b/missioncontrol/v0/files.py index b507a57..415a4a1 100644 --- a/missioncontrol/v0/files.py +++ b/missioncontrol/v0/files.py @@ -10,12 +10,12 @@ def search(**kwargs): kwargs.pop('token_info', None) kwargs.pop('user', None) - start = kwargs.pop('start', None) + start = kwargs.pop('range_start', None) # Some data args = [] if start: args.append(Q(end__gte=start) | (Q(end__isnull=True) & Q(start__gte=start))) - end = kwargs.pop('end', None) + end = kwargs.pop('range_end', None) if end: kwargs['start__lte'] = end results = S3File.objects.filter(*args, **kwargs) From 887838334be0f1922fed2ab9d8fc6baa3cb27e65 Mon Sep 17 00:00:00 2001 From: Louis des Landes Date: Wed, 6 Mar 2019 10:47:51 +1030 Subject: [PATCH 6/8] S3File model: add version and change task-run to work-id --- .../migrations/0022_auto_20190306_0017.py | 33 +++++++++++++++++ .../migrations/0023_auto_20190306_0231.py | 22 ++++++++++++ missioncontrol/home/models.py | 35 ++++++++++++++----- missioncontrol/openapi/openapi.yaml | 19 ++++++---- missioncontrol/tests/conftest.py | 6 +++- missioncontrol/tests/test_files.py | 34 ++++++++++++++---- missioncontrol/v0/files.py | 20 +++++------ 7 files changed, 137 insertions(+), 32 deletions(-) create mode 100644 missioncontrol/home/migrations/0022_auto_20190306_0017.py create mode 100644 missioncontrol/home/migrations/0023_auto_20190306_0231.py diff --git a/missioncontrol/home/migrations/0022_auto_20190306_0017.py b/missioncontrol/home/migrations/0022_auto_20190306_0017.py new file mode 100644 index 0000000..3990147 --- /dev/null +++ b/missioncontrol/home/migrations/0022_auto_20190306_0017.py @@ -0,0 +1,33 @@ +# Generated by Django 2.1.4 on 2019-03-06 00:17 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('home', '0021_auto_20190304_0300'), + ] + + operations = [ + migrations.AddField( + model_name='s3file', + name='version', + field=models.IntegerField(default=1), + preserve_default=False, + ), + migrations.AddField( + model_name='s3file', + name='work_id', + field=models.TextField(default=1), + preserve_default=False, + ), + migrations.RemoveField( + model_name='s3file', + name='task_run', + ), + migrations.AlterUniqueTogether( + name='s3file', + unique_together={('version', 'what', 'where', 'work_id')}, + ), + ] diff --git a/missioncontrol/home/migrations/0023_auto_20190306_0231.py b/missioncontrol/home/migrations/0023_auto_20190306_0231.py new file mode 100644 index 0000000..8dbb8dc --- /dev/null +++ b/missioncontrol/home/migrations/0023_auto_20190306_0231.py @@ -0,0 +1,22 @@ +# Generated by Django 2.1.4 on 2019-03-06 02:31 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('home', '0022_auto_20190306_0017'), + ] + + operations = [ + migrations.AlterModelOptions( + name='s3file', + options={'ordering': ('-version',)}, + ), + migrations.AlterField( + model_name='s3file', + name='work_id', + field=models.TextField(blank=True, null=True), + ), + ] diff --git a/missioncontrol/home/models.py b/missioncontrol/home/models.py index cf95746..6e5fa41 100644 --- a/missioncontrol/home/models.py +++ b/missioncontrol/home/models.py @@ -13,7 +13,7 @@ from django.contrib.postgres.fields import JSONField, HStoreField from django.db import models from django import forms -from django.db.models import Q +from django.db.models import Q, Max from django.db.models.signals import pre_save from django.dispatch import receiver from django.utils import timezone, dateformat @@ -448,11 +448,10 @@ def to_dict(self): return retval def _get_file_cid_if_exists(self, what): - try: - f = self.files.get(what=what) - except S3File.DoesNotExist: - return None - return f.cid + f = S3File.objects.filter(what=what, work_id=self.uuid).first() + if f: + return f.cid + return None @property def stdout(self): @@ -486,8 +485,12 @@ class S3File(models.Model, Serializable): 'Can be blank if instantaneous file.', null=True, blank=True) created = ISODateTimeField(auto_now_add=True) - task_run = models.ForeignKey(TaskRun, to_field='uuid', related_name='files', - on_delete=models.PROTECT, null=True, blank=True) + work_id = models.TextField(null=True, blank=True) + version = models.IntegerField() + + class Meta: + unique_together = ('version', 'what', 'where', 'work_id') + ordering = ('-version', ) # s3://bucket/some_path @property @@ -495,6 +498,7 @@ def prefix(self): path = settings.FILE_STORAGE_PATH if path.startswith('s3://'): return '/'.join(path.split('/')[3:]) + # TODO raise NotImplementedError("Not yet implemented non s3 paths") @property @@ -502,6 +506,7 @@ def bucket(self): path = settings.FILE_STORAGE_PATH if path.startswith('s3://'): return path.split('/')[2] + # TODO raise NotImplementedError("Not yet implemented non s3 paths") @property @@ -529,6 +534,20 @@ def get_upload_url(self): return post + def save(self, *args, **kwargs): + # Set version if not given + if self.version is None: + prev_version = S3File.objects.filter( + what=self.what, where=self.where, work_id=self.work_id + ).aggregate(max_version=Max('version')) + + if prev_version['max_version'] is None: + self.version = 1 + else: + self.version = prev_version['max_version'] + 1 + + super().save(*args, **kwargs) + class CachedAccess(models.Model): diff --git a/missioncontrol/openapi/openapi.yaml b/missioncontrol/openapi/openapi.yaml index 5eab3cd..9c461d2 100644 --- a/missioncontrol/openapi/openapi.yaml +++ b/missioncontrol/openapi/openapi.yaml @@ -1125,9 +1125,9 @@ paths: schema: type: string - in: query - name: task_run + name: work_id description: - Only return files with this task_run uuid. + Only return files with this work_id uuid. schema: type: string format: uuid @@ -1151,6 +1151,14 @@ paths: Only return files with this path name schema: type: string + - in: query + name: version + description: + Only return files with this version number + # TODO add a flag to return just the latest + # version numbers + schema: + type: string responses: 200: description: A list of files @@ -1542,15 +1550,14 @@ components: type: string format: date-time nullable: true - task_run: + work_id: type: string format: uuid nullable: true - post_url_fields: - "$ref": "#/components/schemas/PostUrlFields" + version: + type: integer readOnly: true - PostUrlFields: properties: url: diff --git a/missioncontrol/tests/conftest.py b/missioncontrol/tests/conftest.py index 1115c9e..3ec356c 100644 --- a/missioncontrol/tests/conftest.py +++ b/missioncontrol/tests/conftest.py @@ -140,7 +140,7 @@ def simple_file(some_hash): 'what': 'stdout', 'start': "2018-11-25T01:00:00.000000Z", 'end': None, - 'task_run': None, + 'work_id': None, 'path': '/some/path/for/files', 'where': 'somewhere hidden', } @@ -160,4 +160,8 @@ def yet_another_uuid(): @pytest.fixture def some_hash(): + return hashlib.blake2b(uuid4().bytes).hexdigest() + +@pytest.fixture +def another_hash(): return hashlib.blake2b(uuid4().bytes).hexdigest() \ No newline at end of file diff --git a/missioncontrol/tests/test_files.py b/missioncontrol/tests/test_files.py index cab17ff..e75f1f1 100644 --- a/missioncontrol/tests/test_files.py +++ b/missioncontrol/tests/test_files.py @@ -24,11 +24,10 @@ def test_file_put_get(boto3_mock, test_client, some_hash, simple_file): assert response.status_code == 200, response.get_data() expected = simple_file.copy() - expected['post_url_fields'] = post_values - formatter = dateformat.DateFormat(created) expected['created'] = formatter.format(settings.DATETIME_FORMAT) + expected['version'] = 1 assert response.json == expected try: @@ -70,7 +69,7 @@ def test_file_search_empty(test_client, some_uuid): 'what': 'a thing', 'cid': 'nope', 'where': 'nowhere', - 'task_run': some_uuid, + 'work_id': some_uuid, 'range_start': "2018-11-25T00:00:00.000000Z", 'range_end': "2018-11-25T00:00:00.000000Z", }) @@ -121,7 +120,7 @@ def create_asset(asset_type, asset): json=simple_task_run) assert response.status_code == 201, response.get_data() - simple_file['task_run'] = some_uuid + simple_file['work_id'] = some_uuid created = timezone.now() with patch('django.utils.timezone.now', return_value=created): response = test_client.put(f'/api/v0/files/{some_hash}/', json=simple_file) @@ -130,6 +129,7 @@ def create_asset(asset_type, asset): expected = simple_file.copy() formatter = dateformat.DateFormat(created) expected['created'] = formatter.format(settings.DATETIME_FORMAT) + expected['version'] = 1 search_query = simple_file.copy() search_query['range_start'] = search_query.pop('start') @@ -142,7 +142,7 @@ def create_asset(asset_type, asset): # Make sure changing some of the searches *DON'T* find it. simple_query_false = simple_file.copy() - simple_query_false['task_run'] = uuid.uuid4() + simple_query_false['work_id'] = uuid.uuid4() simple_query_false['range_start'] = simple_query_false.pop('start') response = test_client.get(f'/api/v0/files/', query_string=simple_query_false) assert response.status_code == 200, response.get_data() @@ -159,4 +159,26 @@ def create_asset(asset_type, asset): expected['stdout'] = simple_file['cid'] expected['stderr'] = None - assert response.json == expected \ No newline at end of file + assert response.json == expected + + +@pytest.mark.django_db +def test_version_increment(test_client, simple_file, some_hash, another_hash): + created = timezone.now() + with patch('django.utils.timezone.now', return_value=created): + response = test_client.put(f'/api/v0/files/{some_hash}/', json=simple_file) + assert response.status_code == 201, response.get_data() + result1 = response.json + + simple_file['cid'] = another_hash + with patch('django.utils.timezone.now', return_value=created): + response = test_client.put(f'/api/v0/files/{another_hash}/', json=simple_file) + assert response.status_code == 201, response.get_data() + result2 = response.json + + assert result1.pop('version') == 1 + assert result2.pop('version') == 2 + + assert result1.pop('cid') == some_hash + assert result2.pop('cid') == another_hash + assert result1 == result2 diff --git a/missioncontrol/v0/files.py b/missioncontrol/v0/files.py index 415a4a1..40b98a5 100644 --- a/missioncontrol/v0/files.py +++ b/missioncontrol/v0/files.py @@ -1,9 +1,10 @@ import requests +from connexion.exceptions import ProblemException from django.conf import settings from django.db.models import Q -from home.models import S3File, TaskRun +from home.models import S3File def search(**kwargs): # remove some unused ones @@ -31,19 +32,16 @@ def get_data(cid): def get(cid): obj = S3File.objects.get(cid=cid) retval = obj.to_dict() - retval['post_url_fields'] = obj.get_upload_url() return retval def put(cid, file_body): - task_run_uuid = file_body.pop('task_run', None) - task_run = None - if task_run_uuid: - # See if it exists - task_run = TaskRun.objects.get(uuid=task_run_uuid) - file_body['task_run'] = task_run - + body_cid = file_body.pop('cid', None) + if body_cid is not None and cid != body_cid: + raise ProblemException( + status=409, + title='Conflict', + detail='cid in url does not match body', + ) obj, created = S3File.objects.update_or_create(cid=cid, defaults=file_body) - retval = obj.to_dict() - retval['post_url_fields'] = obj.get_upload_url() return retval, 201 if created else 200 \ No newline at end of file From 1f0c4c592e3cfdd832d50dbbd60d3ae1a39201b2 Mon Sep 17 00:00:00 2001 From: Louis des Landes Date: Wed, 6 Mar 2019 14:03:30 +1030 Subject: [PATCH 7/8] Create separate endpoint for getting file upload fields. --- missioncontrol/home/models.py | 9 ++++++--- missioncontrol/openapi/openapi.yaml | 28 +++++++++++++++++++++++++++- missioncontrol/tests/test_files.py | 23 +++++++++++++++++++++++ missioncontrol/v0/files.py | 11 ++++++++++- 4 files changed, 66 insertions(+), 5 deletions(-) diff --git a/missioncontrol/home/models.py b/missioncontrol/home/models.py index 6e5fa41..3716bd8 100644 --- a/missioncontrol/home/models.py +++ b/missioncontrol/home/models.py @@ -525,11 +525,14 @@ def get_download_url(self): return url - def get_upload_url(self): + @classmethod + def get_post_data_fields(cls, **kwargs): + # Create the object but don't save it + obj = cls(**kwargs) s3 = boto3.client('s3') post = s3.generate_presigned_post( - Bucket=self.bucket, - Key=self.key, + Bucket=obj.bucket, + Key=obj.key, ) return post diff --git a/missioncontrol/openapi/openapi.yaml b/missioncontrol/openapi/openapi.yaml index 9c461d2..298d1e1 100644 --- a/missioncontrol/openapi/openapi.yaml +++ b/missioncontrol/openapi/openapi.yaml @@ -1172,6 +1172,32 @@ paths: application/json: schema: $ref: '#/components/schemas/Error' + /files/get_post_data_fields/: + get: + tags: ['files'] + description: Get the fields required for uploading a new file + operationId: v0.files.get_post_data_fields + parameters: + - in: query + name: cid + required: true + description: + The content ID of a file (blake2b hash) + schema: + type: string + responses: + 200: + description: The fields required for a file upload + content: + application/json: + schema: + $ref: '#/components/schemas/PostDataFields' + default: + description: unexpected error + content: + application/json: + schema: + $ref: '#/components/schemas/Error' /files/{cid}/: get: tags: ['files'] @@ -1558,7 +1584,7 @@ components: type: integer readOnly: true - PostUrlFields: + PostDataFields: properties: url: description: The url to post to diff --git a/missioncontrol/tests/test_files.py b/missioncontrol/tests/test_files.py index e75f1f1..0994396 100644 --- a/missioncontrol/tests/test_files.py +++ b/missioncontrol/tests/test_files.py @@ -182,3 +182,26 @@ def test_version_increment(test_client, simple_file, some_hash, another_hash): assert result1.pop('cid') == some_hash assert result2.pop('cid') == another_hash assert result1 == result2 + +# Still needs a database for the user setup. +@patch('home.models.boto3') +@pytest.mark.django_db +def test_get_post_data_fields(boto3_mock, test_client, some_hash): + post_values = { + 'url': 'https://test.example', + 'url_fields': {}, + } + boto3_mock.client.return_value.generate_presigned_post.return_value = post_values + + response = test_client.get( + f'/api/v0/files/get_post_data_fields/', + query_string={'cid': some_hash} + ) + assert response.status_code == 200, response.get_data() + + assert response.json == post_values + + boto3_mock.client.return_value.generate_presigned_post.assert_called_with( + Bucket=settings.FILE_STORAGE_PATH.split('/')[2], + Key=f'django-file-storage/{some_hash}', + ) \ No newline at end of file diff --git a/missioncontrol/v0/files.py b/missioncontrol/v0/files.py index 40b98a5..a00a496 100644 --- a/missioncontrol/v0/files.py +++ b/missioncontrol/v0/files.py @@ -44,4 +44,13 @@ def put(cid, file_body): ) obj, created = S3File.objects.update_or_create(cid=cid, defaults=file_body) retval = obj.to_dict() - return retval, 201 if created else 200 \ No newline at end of file + return retval, 201 if created else 200 + +def get_post_data_fields(cid): + if S3File.objects.filter(cid=cid).exists(): + raise ProblemException( + status=409, + title='Conflict', + detail='This cid already exists in metadata', + ) + return S3File.get_post_data_fields(cid=cid) From 2a3ddfe343d0b15bf69cf7073c6dd69a49841d73 Mon Sep 17 00:00:00 2001 From: Daniel Grossmann-Kavanagh Date: Thu, 14 Mar 2019 17:44:50 +1100 Subject: [PATCH 8/8] handle NotImplementedError --- missioncontrol/api.py | 5 +++++ missioncontrol/tests/test_files.py | 18 +++++++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/missioncontrol/api.py b/missioncontrol/api.py index eb861e2..617c876 100644 --- a/missioncontrol/api.py +++ b/missioncontrol/api.py @@ -23,6 +23,10 @@ def validation_error(exception): return connexion.FlaskApi.get_response(problem) +def not_implemented_error(exception): + problem = connexion.problem(501, "Not Implemented", str(exception)) + return connexion.FlaskApi.get_response(problem) + class CustomJSONEncoder(JSONEncoder): @@ -45,6 +49,7 @@ def create_app(): app.add_api('openapi.yaml', strict_validation=True) app.add_error_handler(ObjectDoesNotExist, object_does_not_exist) app.add_error_handler(ValidationError, validation_error) + app.add_error_handler(NotImplementedError, not_implemented_error) return app diff --git a/missioncontrol/tests/test_files.py b/missioncontrol/tests/test_files.py index 0994396..ce5e3b4 100644 --- a/missioncontrol/tests/test_files.py +++ b/missioncontrol/tests/test_files.py @@ -45,8 +45,6 @@ def test_file_put_get(boto3_mock, test_client, some_hash, simple_file): assert boto3_mock.mock_calls == [] - - @patch('home.models.boto3') @pytest.mark.django_db def test_file_download(boto3_mock, test_client, simple_file, some_hash): @@ -77,6 +75,7 @@ def test_file_search_empty(test_client, some_uuid): assert response.json == [] + @pytest.mark.django_db def test_file_search_one_key(test_client): response = test_client.get( @@ -88,6 +87,7 @@ def test_file_search_one_key(test_client): assert response.json == [] + @pytest.mark.django_db def test_file_search_required_what(test_client): response = test_client.get( @@ -99,6 +99,7 @@ def test_file_search_required_what(test_client): assert response.json['detail'] == "Missing query parameter 'what'" + @patch('home.models.boto3') @pytest.mark.django_db def test_file_search_after_put(boto_patch, test_client, simple_task_run, @@ -183,6 +184,7 @@ def test_version_increment(test_client, simple_file, some_hash, another_hash): assert result2.pop('cid') == another_hash assert result1 == result2 + # Still needs a database for the user setup. @patch('home.models.boto3') @pytest.mark.django_db @@ -204,4 +206,14 @@ def test_get_post_data_fields(boto3_mock, test_client, some_hash): boto3_mock.client.return_value.generate_presigned_post.assert_called_with( Bucket=settings.FILE_STORAGE_PATH.split('/')[2], Key=f'django-file-storage/{some_hash}', - ) \ No newline at end of file + ) + + +@pytest.mark.django_db +def test_get_post_data_fields_local_not_implemented(settings, test_client, some_hash): + settings.FILE_STORAGE_PATH = '/tmp/' + response = test_client.get( + f'/api/v0/files/get_post_data_fields/', + query_string={'cid': some_hash} + ) + assert response.status_code == 501, response.get_data()