add asset signed url proxy logic for docker compose context #1563
Conversation
local_dispatcher/local_dispatch.py
Outdated
| @@ -19,12 +19,13 @@ | |||
|
|
|||
|
|
|||
| class Service(): | |||
| def __init__(self, name, host, port, path_function, is_default=False): | |||
| def __init__(self, name, host, port, path_function, is_default=False, get_service_path=lambda path : path): | |||
There was a problem hiding this comment.
get_service_path can be used to modify service path. For example, I use it to remove proxy_to_minio portion from the url before sending it to minio. For all as the other servcies the identity function is used so nothing is modified.
| @@ -103,6 +104,8 @@ def get_path_with_params(self, path): | |||
| path_with_params = f"{path}?{urllib.parse.unquote(url_parsed.query)}" | |||
| return path_with_params | |||
|
|
|||
| def get_request_query_params(self): | |||
| return dict(urllib.parse.parse_qsl(urllib.parse.urlsplit(request.url).query)) | |||
There was a problem hiding this comment.
The previous logic that was used to add query params to the request wasn't encoding query params in the url friendly way so it didn't work with minio. get_request_query_params is used to build a dict out of query params so they can be passed as params named argument to requests.requests method.
|
|
||
| DOCKER_COMPOSE_CONTEXT = env_adapter.bool(os.getenv('DOCKER_COMPOSE_CONTEXT', False)) | ||
|
|
||
| if DOCKER_COMPOSE_CONTEXT == True: |
There was a problem hiding this comment.
Add minio proxy if DOCKER_COMPOSE_CONTEXT is set.
| @@ -31,8 +31,14 @@ def build_secure_url(self, blob_name: str, expiration_offset: int = None, bucket | |||
| signed_url = super().build_secure_url(blob_name = blob_name, | |||
| expiration_offset = expiration_offset, | |||
| bucket = bucket) | |||
| return signed_url | |||
| if settings.DOCKER_COMPOSE_CONTEXT == True: | |||
| # note need to proxy request through local dispatcher so users can access from the browser | |||
There was a problem hiding this comment.
replace minio url with a url path to a proxy if DOCKER_COMPOSE_CONTEXT is true
|
Thanks Sergey! |
| @@ -191,7 +211,7 @@ def start(self): | |||
|
|
|||
| self.ingress.app.logger.setLevel(logging.INFO) | |||
|
|
|||
| self.ingress.app.debug = False | |||
| self.ingress.app.debug = True | |||
There was a problem hiding this comment.
@sergzak022 TODO: revert this back to self.ingress.app.debug = False
| @@ -141,6 +142,11 @@ services: | |||
| environment: | |||
| - "MINIO_ROOT_USER=${DIFFGRAM_MINIO_ACCESS_KEY_ID}" | |||
| - "MINIO_ROOT_PASSWORD=${DIFFGRAM_MINIO_ACCESS_KEY_SECRET}" | |||
| healthcheck: | |||
There was a problem hiding this comment.
createbuckets task still fails for me sometimes with mc: <ERROR> Unable to initialize new alias from the provided credentials. Server not initialized, please try again. error. I'm looking into making a better healthcheck to try to address it.
| @@ -11,6 +11,7 @@ services: | |||
| - walrus | |||
| environment: | |||
| DOCKER_CONTEXT: True | |||
| DOCKER_COMPOSE_CONTEXT: True | |||
There was a problem hiding this comment.
dispatcher and default need DOCKER_COMPOSE_CONTEXT in order to proxy minio asset urls through dispatcher
| command: server /export --console-address :9001 | ||
|
|
||
| createbuckets: | ||
| image: minio/mc | ||
| depends_on: | ||
| - minio | ||
| minio: | ||
| condition: service_healthy |
There was a problem hiding this comment.
Updated createbuckets to wait till minio is healthy.
No description provided.