Geventhttpclientmergeconflicts #838

SpencerPinegar · 2018-07-09T20:40:52Z

Forked geventhttpclient #713 , created a new branch, merged master, fixed all merge conflicts (only 1 existed in docs).
Want to get this integrated to master so FastHttpClient can be easily used in master -- PR #713 has been checked and approved by 2 people and this small merge conflict was the final issue in getting this feature merged to master.

…ent (https://github.com/gwik/geventhttpclient/)

…ests work without adding a dependency on having a working C compiler environment set up when installing Locust (since geventhttpclient package doesn't have wheels).

…. The text property function tries to decode the text in the same way as python-requests. This makes the API more similar to HttpSession. Added more tests.

…stHttpSession

… a trailing slash

… requests with FastHttpSession, in order to manually control if a request should result in a success or failure

Improved documentation of arguments for FastHttpSession.request() method.

…_response

… reported as failures

# Conflicts: # docs/writing-a-locustfile.rst

codecov · 2018-07-09T20:43:45Z

Codecov Report

❗ No coverage uploaded for pull request base (master@8b17c28). Click here to learn what that means.
The diff coverage is 92.16%.

@@           Coverage Diff            @@
##             master    #838   +/-   ##
========================================
  Coverage          ?   69.2%           
========================================
  Files             ?      15           
  Lines             ?    1604           
  Branches          ?     245           
========================================
  Hits              ?    1110           
  Misses            ?     437           
  Partials          ?      57

Impacted Files	Coverage Δ
locust/contrib/fasthttp.py	`92.16% <92.16%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b17c28...72beb5b. Read the comment docs.

…the response was very quick (not a failure) and that 0.0 was a valid response time for a local server. Because of this i changes it to assertGreaterEqual.

aldenpeterson-wf · 2018-07-09T21:35:34Z

The test failure was because of this change, btw - https://github.com/locustio/locust/pull/830/files#diff-cfaa22649621e474e8ebc0b0cac90d64R117

aldenpeterson-wf · 2018-07-13T21:36:46Z

@heyman - thoughts on merging this?

solowalker27 · 2019-03-01T17:24:22Z

locust/contrib/fasthttp.py

+        if stream:
+            request_meta["content_size"] = int(response.headers.get("content-length") or 0)
+        else:
+            request_meta["content_size"] = len(response.content or "")


In my testing of this, I ran into an issue with a response's content not being saved properly.

When using FastHttpSession and making a request, if stream=False the response always returns with .content being bytearray(b''). If stream=True, the content is as expected. I'm not sure, but from what I can see the problem seems to stem from here where the content is read into the len() call which then causes the urllib response to release content.

I'm currently working around this by adding request_meta["content"] = response.content before this if stream check, but I'm sure there's a better way to retain content.

solowalker27 · 2019-03-01T20:49:28Z

locust/contrib/fasthttp.py

+class FastHttpSession(object):
+    auth_header = None
+
+    def __init__(self, base_url):


Suggested change

def __init__(self, base_url):

def __init__(self, base_url, **kwargs):

Allow configuration of UserAgent and HTTPClient by accepting and forwarding on **kwargs.

solowalker27 · 2019-03-01T20:56:01Z

locust/contrib/fasthttp.py

+    def __init__(self, base_url):
+        self.base_url = base_url
+        self.cookiejar = CookieJar()
+        self.client = LocustUserAgent(max_retries=1, cookiejar=self.cookiejar)


Suggested change

self.client = LocustUserAgent(max_retries=1, cookiejar=self.cookiejar)

self.client = LocustUserAgent(max_retries=1, cookiejar=self.cookiejar, **kwargs)

Allow configuration of UserAgent and HTTPClient by accepting and forwarding on **kwargs.

sj2208 · 2019-03-26T06:27:07Z

@solowalker27
I have issues when i am using "catch_response" parameter. After passing catch_response=True while making a request, content field in response object is always appearing as None.

Below is sample locust file:

from locust import task, TaskSet, HttpLocust
from locust.contrib.fasthttp import FastHttpLocust

class BenchLocust(FastHttpLocust): # or HttpLocust
min_wait = 0
max_wait = 0
host = "https://docs.locust.io/en/stable"

class task_set(TaskSet):
@task
def index(self):
response = self.client.get("/", name="test" ,catch_response=True)
print response
if response.status_code == 200:
jsonBody = json.loads(response.content)
// Do some stuff here with jsonBody

Issues:

When catch_response=True is passed, then issues appear in fetching the content from response.
res = response.content

When catch_response is not passed, then we are able to easily execute the statement
"response.content" .

Can this be looked into ?

cgoldberg · 2019-04-03T01:15:46Z

@SpencerPinegar can you take a look at the failed test and merge conflict?

FAIL: test_request_connection_error (locust.test.test_fasthttp.TestRequestStatsWithWebserver)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/locustio/locust/locust/test/test_fasthttp.py", line 172, in test_request_connection_error
    self.assertEqual(0, global_stats.get("/", "GET").num_requests)
AssertionError: 0 != 1

skivis · 2019-05-15T15:18:38Z

docs/increase-performance.rst

+The reason for this is that requests is a very well-maintained python package, that 
+provides a really nice API, that many python developers are familiar with. Therefore, 
+in many cases, we recommend that you use the default :py:class:`HttpLocust <locust.core.HttpLocust>` 
+which uses requests. However, if you're planning to run really large scale scale tests, 


Suggested change

which uses requests. However, if you're planning to run really large scale scale tests,

which uses requests. However, if you're planning to run really large scale tests,

skivis · 2019-05-15T16:23:06Z

locust/test/test_fasthttp.py

+        response = locust.client.get("/", timeout=0.1)
+        self.assertEqual(response.status_code, 0)
+        self.assertEqual(1, global_stats.get("/", "GET").num_failures)
+        self.assertEqual(0, global_stats.get("/", "GET").num_requests)


Since 0.10.0 (see #939), both successful and failed requests are counted in total number of requests.

Suggested change

self.assertEqual(0, global_stats.get("/", "GET").num_requests)

self.assertEqual(1, global_stats.get("/", "GET").num_requests)

I really don't mean to be rude, but can we get this merged any time soon? Are there any more issues that need to be solved, I'd be happy to take a look in that case.

Cheers!

cgoldberg · 2019-05-15T18:34:07Z

Are there any more issues that need to be solved,
I'd be happy to take a look

there are merge conflicts.. have at them

sj2208 · 2019-05-21T05:52:36Z

@solowalker27
I have issues when i am using "catch_response" parameter. After passing catch_response=True while making a request, content field in response object is always appearing as None.

Below is sample locust file:

from locust import task, TaskSet, HttpLocust
from locust.contrib.fasthttp import FastHttpLocust

class BenchLocust(FastHttpLocust): # or HttpLocust
min_wait = 0
max_wait = 0
host = "https://docs.locust.io/en/stable"

class task_set(TaskSet):
@task
def index(self):
response = self.client.get("/", name="test" ,catch_response=True)
print response
if response.status_code == 200:
jsonBody = json.loads(response.content)
// Do some stuff here with jsonBody

Issues:

When catch_response=True is passed, then issues appear in fetching the content from response.
res = response.content

When catch_response is not passed, then we are able to easily execute the statement
"response.content" .

Can this be looked into ?

@cgoldberg @solowalker27 -

heyman and others added 29 commits December 19, 2017 15:22

Started implementing a new Locust HTTP client that uses geventhttpcli…

9a0bc33

…ent (https://github.com/gwik/geventhttpclient/)

Added geventhttpclient to tests_require in setup.py. This makes the t…

5de56c9

…ests work without adding a dependency on having a working C compiler environment set up when installing Locust (since geventhttpclient package doesn't have wheels).

Added geventhttpclient to tox.ini

9fd6621

Python 2 fixes

0a8af7b

Use timeit.default_timer() instead of time.time()

84bb988

Added text property to Response objects returned by GeventHttpSession…

255c76c

…. The text property function tries to decode the text in the same way as python-requests. This makes the API more similar to HttpSession. Added more tests.

Fixed wrong link target

cce8c66

Renamed GeventHttpLocust -> FasHttpLocust and GeventHttpSession -> Fa…

0351db8

…stHttpSession

Renamed files

f1e5761

Fixed import

a33a4f5

Renamed response class

7fb8dd5

Added FastHttpSession.patch()

c5e0618

Started writing docs for FastHttpLocust

5f464a1

Updated test that accidentally broke

5116f30

Minor RST syntax fix

9ed5623

Check that the given FastHttpLocust.host is valid and doesn't contain…

e8c87ac

… a trailing slash

Implemented support for using the catch_response argument when making…

0f0d047

… requests with FastHttpSession, in order to manually control if a request should result in a success or failure

Implemented support for HTTP Basic Auth in FastHttp client.

064f5e0

Improved documentation of arguments for FastHttpSession.request() method.

Typo

748ba8d

Fixed tests for manually failing an already failed request with catch…

a1a42f0

…_response

Removed commented out code

03d436e

Clarified documentation about performance gains of FastHttpClient

42bfffb

Added HTTPConnectionClosed to "whitelisted" exceptions that should be…

a8b8abd

… reported as failures

Spelling typo

84b0579

Removed "Known limitations" (yay)

f34dcac

Merge branch 'master' into geventhttpclient

f1dad45

Added dependency on PyPI package geventhttpclient-wheels

7055334

Removed geventhttpclient from tox dependencies

40aebc6

Merge branch 'master' into geventhttpclientmergeconflicts

a36adaf

# Conflicts: # docs/writing-a-locustfile.rst

I was getting the assertionError 0.0 not greater than 0 -- I assumed …

1171b9b

…the response was very quick (not a failure) and that 0.0 was a valid response time for a local server. Because of this i changes it to assertGreaterEqual.

solowalker27 reviewed Mar 1, 2019

View reviewed changes

Merge branch 'master' into geventhttpclientmergeconflicts

72beb5b

tylerturk mentioned this pull request Apr 3, 2019

Added FastHttpLocust with HTTP client that uses geventhttpclient #713

Closed

skivis reviewed May 15, 2019

View reviewed changes

skivis mentioned this pull request May 16, 2019

FastHttpLocust #1014

Merged

mbeacom merged commit 72beb5b into locustio:master Sep 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Geventhttpclientmergeconflicts #838

Geventhttpclientmergeconflicts #838

SpencerPinegar commented Jul 9, 2018 •

edited

Loading

codecov bot commented Jul 9, 2018 •

edited

Loading

aldenpeterson-wf commented Jul 9, 2018

aldenpeterson-wf commented Jul 13, 2018

solowalker27 Mar 1, 2019

solowalker27 Mar 1, 2019 •

edited

Loading

solowalker27 Mar 1, 2019

sj2208 commented Mar 26, 2019 •

edited

Loading

cgoldberg commented Apr 3, 2019

skivis May 15, 2019

skivis May 15, 2019

cgoldberg commented May 15, 2019

sj2208 commented May 21, 2019

	def __init__(self, base_url):
	def __init__(self, base_url, **kwargs):

	self.client = LocustUserAgent(max_retries=1, cookiejar=self.cookiejar)
	self.client = LocustUserAgent(max_retries=1, cookiejar=self.cookiejar, **kwargs)

	which uses requests. However, if you're planning to run really large scale scale tests,
	which uses requests. However, if you're planning to run really large scale tests,

	self.assertEqual(0, global_stats.get("/", "GET").num_requests)
	self.assertEqual(1, global_stats.get("/", "GET").num_requests)

Geventhttpclientmergeconflicts #838

Geventhttpclientmergeconflicts #838

Conversation

SpencerPinegar commented Jul 9, 2018 • edited Loading

codecov bot commented Jul 9, 2018 • edited Loading

Codecov Report

aldenpeterson-wf commented Jul 9, 2018

aldenpeterson-wf commented Jul 13, 2018

solowalker27 Mar 1, 2019

Choose a reason for hiding this comment

solowalker27 Mar 1, 2019 • edited Loading

Choose a reason for hiding this comment

solowalker27 Mar 1, 2019

Choose a reason for hiding this comment

sj2208 commented Mar 26, 2019 • edited Loading

cgoldberg commented Apr 3, 2019

skivis May 15, 2019

Choose a reason for hiding this comment

skivis May 15, 2019

Choose a reason for hiding this comment

cgoldberg commented May 15, 2019

sj2208 commented May 21, 2019

SpencerPinegar commented Jul 9, 2018 •

edited

Loading

codecov bot commented Jul 9, 2018 •

edited

Loading

solowalker27 Mar 1, 2019 •

edited

Loading

sj2208 commented Mar 26, 2019 •

edited

Loading