zhongjiajie commented on code in PR #130:
URL:
https://github.com/apache/dolphinscheduler-sdk-python/pull/130#discussion_r1440222676
##########
src/pydolphinscheduler/tasks/http.py:
##########
@@ -82,8 +106,27 @@ def __init__(
raise PyDSParamException(
"Parameter http_method %s not support.", http_method
)
+
+ if isinstance(http_params, list):
+ warnings.warn(
+ "The `http_params` parameter currently accepts a dictionary
instead of a list. Your parameter is being ignored.",
+ DeprecationWarning,
+ )
+
self.http_method = http_method
- self.http_params = http_params or []
+ self.http_params = http_params # Storing the original value
Review Comment:
directly use param name `_http_params` is better, because we can remove the
`@http_params.setter`
```suggestion
self._http_params = http_params
```
##########
src/pydolphinscheduler/tasks/http.py:
##########
@@ -51,7 +53,29 @@ class HttpCheckCondition:
class Http(Task):
- """Task HTTP object, declare behavior for HTTP task to dolphinscheduler."""
+ """Task HTTP object, declare behavior for HTTP task to dolphinscheduler.
+ Attributes:
+ _task_custom_attr (set): A set containing custom attributes specific
to the Http task,
+ including 'url', 'http_method', 'http_params',
and more.
+
+ Args:
+ name (str): The name or identifier for the HTTP task.
+ url (str): The URL endpoint for the HTTP request.
+ http_method (str, optional): The HTTP method for the request (GET,
POST, etc.). Defaults to HttpMethod.GET.
+ http_params (str, optional): Parameters for the HTTP request. Defaults
to None.
+ http_check_condition (str, optional): Condition for checking the HTTP
response status.
+ Defaults to
HttpCheckCondition.STATUS_CODE_DEFAULT.
+ condition (str, optional): Additional condition to evaluate if
`http_check_condition` is not STATUS_CODE_DEFAULT.
+ connect_timeout (int, optional): Connection timeout for the HTTP
request in milliseconds. Defaults to 60000.
+ socket_timeout (int, optional): Socket timeout for the HTTP request in
milliseconds. Defaults to 60000.
+
+ Raises:
+ PyDSParamException: Exception raised for invalid parameters, such as
unsupported HTTP methods or conditions.
+
+ Example:
+ Usage example for creating an HTTP task:
+ http_task = Http(name="http_task", url="https://api.example.com";,
http_method="POST", http_params={"key": "value"})
Review Comment:
you add a good example 👍
##########
src/pydolphinscheduler/tasks/http.py:
##########
@@ -51,7 +53,29 @@ class HttpCheckCondition:
class Http(Task):
- """Task HTTP object, declare behavior for HTTP task to dolphinscheduler."""
+ """Task HTTP object, declare behavior for HTTP task to dolphinscheduler.
+ Attributes:
+ _task_custom_attr (set): A set containing custom attributes specific
to the Http task,
+ including 'url', 'http_method', 'http_params',
and more.
+
+ Args:
+ name (str): The name or identifier for the HTTP task.
+ url (str): The URL endpoint for the HTTP request.
+ http_method (str, optional): The HTTP method for the request (GET,
POST, etc.). Defaults to HttpMethod.GET.
+ http_params (str, optional): Parameters for the HTTP request. Defaults
to None.
+ http_check_condition (str, optional): Condition for checking the HTTP
response status.
+ Defaults to
HttpCheckCondition.STATUS_CODE_DEFAULT.
+ condition (str, optional): Additional condition to evaluate if
`http_check_condition` is not STATUS_CODE_DEFAULT.
+ connect_timeout (int, optional): Connection timeout for the HTTP
request in milliseconds. Defaults to 60000.
+ socket_timeout (int, optional): Socket timeout for the HTTP request in
milliseconds. Defaults to 60000.
Review Comment:
can we change the format to
```py
:param name: A unique, meaningful string for the shell task.
:param command: One or more command want to run in this task.
```
just like we did in shell task, in this format our docs template can auto
fill the parameter type for our users
##########
src/pydolphinscheduler/tasks/test_http_conversion.py:
##########
@@ -0,0 +1,32 @@
+from pydolphinscheduler.core.parameter import ParameterHelper, ParameterType
+from http.py import Http # Replace 'your_module_path' with the actual path or
import structure
+
+def test_http_params_conversion():
Review Comment:
Great! you add some tests for this, but we already have tests for task http
in
https://github.com/apache/dolphinscheduler-sdk-python/blob/f82e07d232380fe9f5582db4dcca920ae00befd4/tests/tasks/test_http.py
could you merge into that file?
##########
src/pydolphinscheduler/tasks/http.py:
##########
@@ -82,8 +106,27 @@ def __init__(
raise PyDSParamException(
"Parameter http_method %s not support.", http_method
)
+
+ if isinstance(http_params, list):
+ warnings.warn(
+ "The `http_params` parameter currently accepts a dictionary
instead of a list. Your parameter is being ignored.",
+ DeprecationWarning,
+ )
+
self.http_method = http_method
- self.http_params = http_params or []
+ self.http_params = http_params # Storing the original value
+
+ @property
+ def http_params(self):
+ """Property to convert http_params using ParameterHelper when
accessed."""
+ return ParameterHelper.convert_params(self._http_params,
direction=Direction.IN) if self._http_params else []
+
+ @http_params.setter
+ def http_params(self, value):
+ """Setter method for http_params."""
+ self._http_params = value
+
+
Review Comment:
remove because we directly use param `_http_params`
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
