-
Notifications
You must be signed in to change notification settings - Fork 1
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
modification for reaxflow-workflow-service #70
base: main
Are you sure you want to change the base?
Changes from 7 commits
822ed8c
0f01a52
0581c4e
d79d052
4be4c9f
a5f11e4
8dd7267
c2907eb
f1d6efe
ee34679
832aeaf
b579abe
4a57eed
a83c96c
293ed92
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 |
---|---|---|
@@ -1,6 +1,9 @@ | ||
from .object_storage import MarketPlaceObjectStorageApp | ||
from .system import MarketPlaceSystemApp | ||
from .transformation import MarketPlaceTransformationApp | ||
|
||
|
||
class MarketPlaceApp(MarketPlaceObjectStorageApp, MarketPlaceTransformationApp): | ||
class MarketPlaceApp( | ||
MarketPlaceObjectStorageApp, MarketPlaceTransformationApp, MarketPlaceSystemApp | ||
): | ||
pass |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
from typing import Optional | ||
|
||
from fastapi.responses import JSONResponse, Response | ||
|
||
from ..utils import check_capability_availability | ||
from .base import _MarketPlaceAppBase | ||
|
||
|
||
class MarketPlaceSystemApp(_MarketPlaceAppBase): | ||
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. Why aren't these capabilities directly defined in 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 makes sense. Since there was an additional system level file in standardapp, I added a new one here. I will move them to base. |
||
@check_capability_availability | ||
def get_logs( | ||
self, id: Optional[str], limit: int = 100, offset: int = 0 | ||
) -> Response: | ||
return self._client.get( | ||
self._proxy_path("getLogs"), | ||
params={"id": id, "limit": limit, "offset": offset}, | ||
).content | ||
|
||
@check_capability_availability | ||
def get_info(self, config: dict = None) -> JSONResponse: | ||
params = {} | ||
if config is not None: | ||
params.update(config) | ||
return self._client.get(self._proxy_path("getInfo"), params=params).json() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,11 +18,19 @@ def get_transformation_list( | |
|
||
@check_capability_availability | ||
def new_transformation( | ||
self, new_transformation: transformation.NewTransformationModel | ||
self, | ||
new_transformation: transformation.NewTransformationModel, | ||
config: dict = None, | ||
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 purpose of 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. To send additional query parameters which are not defined in Marketplace. For example reaxpro-service needs model_name an additional query parameter to create the transformation. Any key values that are passed in config will be send as query parameters to the application by marketplace. An alternative solution can be to send inside the body. I thought sending it as query will make it less restriction at the application side. 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 still think needs a proper docstring (so users can see it through contextual help in their IDEs) would be specially benefitial in this instance. |
||
) -> transformation.TransformationCreateResponse: | ||
params = {} | ||
# send additional key value as query parameters if some app needs it | ||
if config is not None: | ||
params.update(config) | ||
return transformation.TransformationCreateResponse.parse_obj( | ||
self._client.post( | ||
self._proxy_path("newTransformation"), json=new_transformation | ||
self._proxy_path("newTransformation"), | ||
json=new_transformation, | ||
params=params, | ||
).json() | ||
) | ||
|
||
|
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.
Why isn't the default defined at the Standard app API level?
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 thought it would be easy to integrate if in case we need a name change for default collection. SInce standardapp requires rebuilding production. But anyway now I think it soesnot make sense to change the name once datasink application is established. I can move it to standard app if needed.