-
Notifications
You must be signed in to change notification settings - Fork 18
Initial Dataset Management GUI #197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
020c12e
9901d4f
371cd56
df3d259
2777322
13486ba
50b0ba1
853a8c8
7f4abae
8a05d71
274719b
4f6da15
a65ddfe
c403de7
ca81028
f823d81
cb61094
0199609
98ccf92
6670dd1
0f0cd00
fc13804
2afa9bd
edd2622
0d0ad3a
a6b0aa6
354d3c5
9916b4d
14f675f
57632b1
cf2256a
ced58ed
09afa58
5f0384d
45303a9
e98ef36
3a7e39f
2c44f7e
ea10c34
9717569
8ce767f
e47e730
9831d00
7d81da7
953cfdd
508a0ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| from abc import ABC | ||
| from django.views.generic.base import View | ||
| from dmod.client.request_clients import DatasetExternalClient | ||
| import logging | ||
| logger = logging.getLogger("gui_log") | ||
| from .DMODProxy import DMODMixin, GUI_STATIC_SSL_DIR | ||
| from typing import Dict | ||
|
|
||
|
|
||
| class AbstractDatasetView(View, DMODMixin, ABC): | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| super(AbstractDatasetView, self).__init__(*args, **kwargs) | ||
| self._dataset_client = None | ||
|
|
||
| async def get_dataset(self, dataset_name: str) -> Dict[str, dict]: | ||
| serial_dataset = await self.dataset_client.get_serialized_datasets(dataset_name=dataset_name) | ||
| return serial_dataset | ||
|
|
||
| async def get_datasets(self) -> Dict[str, dict]: | ||
| serial_datasets = await self.dataset_client.get_serialized_datasets() | ||
| return serial_datasets | ||
|
|
||
| @property | ||
| def dataset_client(self) -> DatasetExternalClient: | ||
| if self._dataset_client is None: | ||
| self._dataset_client = DatasetExternalClient(endpoint_uri=self.maas_endpoint_uri, | ||
| ssl_directory=GUI_STATIC_SSL_DIR) | ||
| return self._dataset_client | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,8 @@ | |
| from pathlib import Path | ||
| from typing import List, Optional, Tuple, Type | ||
|
|
||
| GUI_STATIC_SSL_DIR = Path('/usr/maas_portal/ssl') | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we might want to mark this as a TODO in the future to make this configurable.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should also be handled by Django as well. Shouldn't need to be too necessary. Should be in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! Thanks for catching this. A comment noting this implicit behavior would be appreciated!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For some reason I thought I put that in settings.py. |
||
|
|
||
|
|
||
| class RequestFormProcessor(ABC): | ||
|
|
||
|
|
@@ -209,7 +211,7 @@ class PostFormRequestClient(ModelExecRequestClient): | |
| def _bootstrap_ssl_dir(cls, ssl_dir: Optional[Path] = None): | ||
| if ssl_dir is None: | ||
| ssl_dir = Path(__file__).resolve().parent.parent.parent.joinpath('ssl') | ||
| ssl_dir = Path('/usr/maas_portal/ssl') #Fixme | ||
| ssl_dir = GUI_STATIC_SSL_DIR #Fixme | ||
| return ssl_dir | ||
|
|
||
| def __init__(self, endpoint_uri: str, http_request: HttpRequest, ssl_dir: Optional[Path] = None): | ||
|
|
@@ -315,6 +317,7 @@ def forward_request(self, request: HttpRequest, event_type: MessageEventType) -> | |
| client = PostFormRequestClient(endpoint_uri=self.maas_endpoint_uri, http_request=request) | ||
| if event_type == MessageEventType.MODEL_EXEC_REQUEST: | ||
| form_processor_type = ModelExecRequestFormProcessor | ||
| # TODO: need a new type of form processor here (or 3 more, for management, uploading, and downloading) | ||
| else: | ||
| raise RuntimeError("{} got unsupported event type: {}".format(self.__class__.__name__, str(event_type))) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import asyncio | ||
| from django.http import JsonResponse | ||
| from wsgiref.util import FileWrapper | ||
| from django.http.response import StreamingHttpResponse | ||
| from .AbstractDatasetView import AbstractDatasetView | ||
| from .DatasetFileWebsocketFilelike import DatasetFileWebsocketFilelike | ||
| import logging | ||
| logger = logging.getLogger("gui_log") | ||
|
|
||
|
|
||
| class DatasetApiView(AbstractDatasetView): | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| super().__init__(*args, **kwargs) | ||
|
|
||
| def _get_dataset_content_details(self, dataset_name: str): | ||
| result = asyncio.get_event_loop().run_until_complete(self.dataset_client.get_dataset_content_details(name=dataset_name)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like there are several calls to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The base functions for sending data through websockets are (and I am pretty sure must be)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems we might want to wrap these methods (all methods with leading |
||
| logger.info(result) | ||
| return JsonResponse({"contents": result}, status=200) | ||
|
|
||
| def _delete_dataset(self, dataset_name: str) -> JsonResponse: | ||
| result = asyncio.get_event_loop().run_until_complete(self.dataset_client.delete_dataset(name=dataset_name)) | ||
| return JsonResponse({"successful": result}, status=200) | ||
|
|
||
| def _get_datasets_json(self) -> JsonResponse: | ||
| serial_dataset_map = asyncio.get_event_loop().run_until_complete(self.get_datasets()) | ||
| return JsonResponse({"datasets": serial_dataset_map}, status=200) | ||
|
|
||
| def _get_dataset_json(self, dataset_name: str) -> JsonResponse: | ||
| serial_dataset = asyncio.get_event_loop().run_until_complete(self.get_dataset(dataset_name=dataset_name)) | ||
| return JsonResponse({"dataset": serial_dataset[dataset_name]}, status=200) | ||
|
|
||
| def _get_download(self, request, *args, **kwargs): | ||
| dataset_name = request.GET.get("dataset_name", None) | ||
| item_name = request.GET.get("item_name", None) | ||
| chunk_size = 8192 | ||
|
|
||
| custom_filelike = DatasetFileWebsocketFilelike(self.dataset_client, dataset_name, item_name) | ||
|
|
||
| response = StreamingHttpResponse( | ||
| FileWrapper(custom_filelike, chunk_size), | ||
| content_type="application/octet-stream" | ||
| ) | ||
| response['Content-Length'] = asyncio.get_event_loop().run_until_complete(self.dataset_client.get_item_size(dataset_name, item_name)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have this part working anyway, so it can be implemented differently. I can pull this and other such things out entirely, but I wasn't sure if parts might be useful to show the direction I was trying to go.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's precisely why it should be left in for now. I just wanted to drop some notes while I was looking at it. I don't think any of us has the bandwidth to make major changes at the moment. |
||
| response['Content-Disposition'] = "attachment; filename=%s" % item_name | ||
| return response | ||
|
|
||
| def get(self, request, *args, **kwargs): | ||
| request_type = request.GET.get("request_type", None) | ||
| if request_type == 'download_file': | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did originally start to do that, but it occurred to me that this kind of violated the intent of having only a single websocket (at least per authenticated session) open for all client communication. In fairness, that isn't working properly at the moment, but I think we want to try to have it that way eventually, or else we'd move to REST. And if we are eventually going for that, I don't think we want too many different views (i.e., because eventually we'll have to consolidate them), although I could be assessing that incorrectly.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, if there's a plan to consolidate splitting them out doesn't make a whole lot of sense. |
||
| return self._get_download(request) | ||
| elif request_type == 'datasets': | ||
| return self._get_datasets_json() | ||
| elif request_type == 'dataset': | ||
| return self._get_dataset_json(dataset_name=request.GET.get("name", None)) | ||
| elif request_type == 'contents': | ||
| return self._get_dataset_content_details(dataset_name=request.GET.get("name", None)) | ||
| if request_type == 'delete': | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could probably be handled through a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, but that should eventually get protected by our authentication and authorization mechanisms. And then this relates to my other comment about how we (probably) want to consolidate views now, or will eventually, for how we want our websocket communication to work. |
||
| return self._delete_dataset(dataset_name=request.GET.get("name", None)) | ||
|
|
||
| # TODO: finish | ||
| return JsonResponse({}, status=400) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Django has a specialized response that might help here at
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ^^ |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import asyncio | ||
| from typing import AnyStr | ||
| from dmod.client.request_clients import DatasetExternalClient | ||
|
|
||
|
|
||
| class DatasetFileWebsocketFilelike: | ||
|
|
||
| def __init__(self, client: DatasetExternalClient, dataset_name: str, file_name: str): | ||
| self._client = client | ||
| self._dataset_name = dataset_name | ||
| self._file_name = file_name | ||
| self._read_index: int = 0 | ||
|
|
||
| def read(self, blksize: int) -> AnyStr: | ||
|
|
||
| result = asyncio.get_event_loop().run_until_complete( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to try catch here? If the |
||
| self._client.download_item_block(dataset_name=self._dataset_name, item_name=self._file_name, | ||
| blk_start=self._read_index, blk_size=blksize)) | ||
| self._read_index += blksize | ||
| return result | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| """ | ||
| Defines a view that may be used to configure a MaaS request | ||
| """ | ||
| import asyncio | ||
| from django.http import HttpRequest, HttpResponse | ||
| from django.shortcuts import render | ||
|
|
||
| import dmod.communication as communication | ||
| from dmod.core.meta_data import DataCategory, DataFormat | ||
|
|
||
| import logging | ||
| logger = logging.getLogger("gui_log") | ||
|
|
||
| from .utils import extract_log_data | ||
| from .AbstractDatasetView import AbstractDatasetView | ||
|
|
||
|
|
||
| class DatasetManagementView(AbstractDatasetView): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thoughts on adding a |
||
|
|
||
| """ | ||
| A view used to configure a dataset management request or requests for transmitting dataset data. | ||
| """ | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| super().__init__(*args, **kwargs) | ||
|
|
||
| def _process_event_type(self, http_request: HttpRequest) -> communication.MessageEventType: | ||
| """ | ||
| Determine and return whether this request is for a ``DATASET_MANAGEMENT`` or ``DATA_TRANSMISSION`` event. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| http_request : HttpRequest | ||
| The raw HTTP request in question. | ||
|
|
||
| Returns | ||
| ------- | ||
| communication.MessageEventType | ||
| Either ``communication.MessageEventType.DATASET_MANAGEMENT`` or | ||
| ``communication.MessageEventType.DATA_TRANSMISSION``. | ||
| """ | ||
| # TODO: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Issue to track this? Or will this be completed in this PR? |
||
| raise NotImplementedError("{}._process_event_type not implemented".format(self.__class__.__name__)) | ||
|
|
||
| def get(self, http_request: HttpRequest, *args, **kwargs) -> HttpResponse: | ||
| """ | ||
| The handler for 'get' requests. | ||
|
|
||
| This will render the 'maas/dataset_management.html' template after retrieving necessary information to initially | ||
| populate the forms it displays. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| http_request : HttpRequest | ||
| The request asking to render this page. | ||
| args | ||
| kwargs | ||
|
|
||
| Returns | ||
| ------- | ||
| A rendered page. | ||
| """ | ||
| errors, warnings, info = extract_log_data(kwargs) | ||
|
|
||
| # Gather map of serialized datasets, keyed by dataset name | ||
| serial_dataset_map = asyncio.get_event_loop().run_until_complete(self.get_datasets()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess we can, but that seems cluttered. Not that I haven't already done that kind of cluttering in places ... Do you think that would just enhance developer usability of the client class, or were there other advantages you were anticipating?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know that I'm the type of goober that would end up calling |
||
| serial_dataset_list = [serial_dataset_map[d] for d in serial_dataset_map] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is nit-picky, but for code readability, |
||
|
|
||
| dataset_categories = [c.name.title() for c in DataCategory] | ||
| dataset_formats = [f.name for f in DataFormat] | ||
|
|
||
| payload = { | ||
| 'datasets': serial_dataset_list, | ||
| 'dataset_categories': dataset_categories, | ||
| 'dataset_formats': dataset_formats, | ||
| 'errors': errors, | ||
| 'info': info, | ||
| 'warnings': warnings | ||
| } | ||
|
|
||
| return render(http_request, 'maas/dataset_management.html', payload) | ||
|
|
||
| def post(self, http_request: HttpRequest, *args, **kwargs) -> HttpResponse: | ||
| """ | ||
| The handler for 'post' requests. | ||
|
|
||
| This will attempt to submit the request and rerender the page like a 'get' request. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| http_request : HttpRequest | ||
| The request asking to render this page. | ||
| args | ||
| kwargs | ||
|
|
||
| Returns | ||
| ------- | ||
| A rendered page. | ||
| """ | ||
| # TODO: implement this to figure out whether DATASET_MANAGEMENT or DATA_TRANSMISSION | ||
| event_type = self._process_event_type(http_request) | ||
| client, session_data, dmod_response = self.forward_request(http_request, event_type) | ||
|
|
||
| # TODO: this probably isn't exactly correct, so review once closer to completion | ||
| if dmod_response is not None and 'dataset_id' in dmod_response.data: | ||
| session_data['new_dataset_id'] = dmod_response.data['dataset_id'] | ||
|
|
||
| http_response = self.get(http_request=http_request, errors=client.errors, warnings=client.warnings, | ||
| info=client.info, *args, **kwargs) | ||
|
|
||
| for k, v in session_data.items(): | ||
| http_response.set_cookie(k, v) | ||
|
|
||
| return http_response | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get the desire to add blanket implementation here for downstream classes, but given that all methods and properties are implemented, is the use of abstract here to indicate to downstream users that they should inherit from this? Still coming up the speed on the codebase so this might be an idiom ive not picked up on yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Essentially yes. In particular, to reusably abstract a view that has a dataset client property. But it's not especially meaningful on it's own.