Skip to content
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

More pythonic HTTP task parameter #88

Closed
zhongjiajie opened this issue Apr 4, 2023 · 7 comments · Fixed by #130
Closed

More pythonic HTTP task parameter #88

zhongjiajie opened this issue Apr 4, 2023 · 7 comments · Fixed by #130
Assignees
Labels
good first issue Good for newcomers improvement Improve exists function pri:high priority high
Milestone

Comments

@zhongjiajie
Copy link
Member

currently like this

http = Http(
        name="http",
        url="http://www.google.com",
        http_method="GET",
        http_params=[
            {"prop": "abc", "httpParametersType": "PARAMETER", "value": "def"}
        ],
    )

should be more pythonic

@zhongjiajie zhongjiajie added the improvement Improve exists function label Apr 4, 2023
@zhongjiajie zhongjiajie self-assigned this Apr 4, 2023
@zhongjiajie zhongjiajie added the pri:high priority high label Apr 4, 2023
@zhongjiajie zhongjiajie removed their assignment May 5, 2023
@zhongjiajie zhongjiajie added the good first issue Good for newcomers label May 5, 2023
@zhongjiajie zhongjiajie added this to the 4.0.4 milestone Sep 8, 2023
@zhongjiajie zhongjiajie modified the milestones: 4.0.4, 4.1.0 Oct 12, 2023
@HarshitNagpal29
Copy link
Contributor

@zhongjiajie sir i tried to transform your given snippet of code in more pythonic manner you can see below, if you still want some changes then please tell me , also provide your valuable feedback as well as i am new to to open source community.

`from typing import List, Dict

class Http:
"""Represents an HTTP request object."""

def __init__(self, name: str, url: str, http_method: str, http_params: List[Dict[str, str]]):
    self.name = name
    self.url = url
    self.http_method = http_method
    self.http_params = http_params

http_params = [
{"prop": "abc", "http_parameters_type": "PARAMETER", "value": "def"},
{"prop": "ghi", "http_parameters_type": "HEADER", "value": "jkl"},

]

http = Http(name="http", url="http://www.google.com", http_method="GET", http_params=http_params)`

**Changes i made: **

Introduced List[Dict[str, str]] which provides clear information that http_params should be a list of dictionaries where each dictionary has string keys and string values.

The init method is explicitly defined within the Http class.

And then some minor changes here and there just to increase readability of code and make it more pythonic.

Thank You.

@zhongjiajie
Copy link
Member Author

hi @HarshitNagpal29 , we already have some utils function for parameter which you can see in

and you can see when we add this, we can use param as dict type

task = Task(
    input_params={"key1": value1}
    output_params={"key2": value2}
)

so maybe we can change http code like

http = Http(
        name="http",
        url="http://www.google.com",
        http_method="GET",
-        http_params=[
-            {"prop": "abc", "httpParametersType": "PARAMETER", "value": "def"}
-        ],
+.    http_params={"abc": "def"}
    )

BTW, we have some test case

def test_convert_params():
"""Test the ParameterHelper convert_params function."""
params = {
"value_INTEGER": 123,
"value_LONG": ParameterType.LONG("1000000"),
}
results = ParameterHelper.convert_params(params, direction=Direction.IN)
expect = [
{"prop": "value_INTEGER", "direct": "IN", "type": "INTEGER", "value": 123},
{"prop": "value_LONG", "direct": "IN", "type": "LONG", "value": "1000000"},
]
assert results == expect
results = ParameterHelper.convert_params(params, direction=Direction.OUT)
expect = [
{"prop": "value_INTEGER", "direct": "OUT", "type": "INTEGER", "value": 123},
{"prop": "value_LONG", "direct": "OUT", "type": "LONG", "value": "1000000"},
]
assert results == expect
maybe you can have a look

@zhongjiajie
Copy link
Member Author

And do you interested in this issue, I can assign you if you interested @HarshitNagpal29

@HarshitNagpal29
Copy link
Contributor

@zhongjiajie ok sir assign me i will try to solve it and make pull request

@zhongjiajie
Copy link
Member Author

I had assigned to you for this issue, looking forward your contribution

@HarshitNagpal29
Copy link
Contributor

sir i have created a pull requested which is waiting for approval

@HarshitNagpal29
Copy link
Contributor

@zhongjiajie sir i have made all the changes which you told me please review it once and if there is still some issue then you can tell me again , thank you for your patience.
7e84357

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers improvement Improve exists function pri:high priority high
Projects
None yet
2 participants