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

Add tasks sequence support #827

Merged
merged 3 commits into from Jun 28, 2018

Conversation

Projects
None yet
5 participants
@Ramshell
Contributor

Ramshell commented Jun 23, 2018

This pr is related to this issue.

As I said In slack, I know there is an easy workaround of defining only one task in the TaskSet and defining all the behavior there. The problem with the workaround is that it is a bit hacky, since you have to do self.wait() whenever it's needed instead of letting the framework handle it.

If accepted this pr will

Introduce a new class: TaskSequence which accepts a new decorator @seq_task(int) to define the order of the tasks.

For example:

class BrowseDocumentationSequence(TaskSquence):
    def on_start(self):
        self.urls_on_current_page = self.toc_urls

    # assume all users arrive at the index page
    @seq_task(1)
    def index_page(self):
        r = self.client.get("/")
        pq = PyQuery(r.content)
        link_elements = pq(".toctree-wrapper a.internal")
        self.toc_urls = [
            l.attrib["href"] for l in link_elements
        ]

    @seq_task(2)
    @task(50)
    def load_page(self, url=None):
        url = random.choice(self.toc_urls)
        r = self.client.get(url)
        pq = PyQuery(r.content)
        link_elements = pq("a.internal")
        self.urls_on_current_page = [
            l.attrib["href"] for l in link_elements
        ]

    @seq_task(3)
    @task(30)
    def load_sub_page(self):
        url = random.choice(self.urls_on_current_page)
        r = self.client.get(url)

In the example above, index_page/1 will be executed first, then load_page/1 fifty times, and then load_sub_page/1 thirty times.

@seq_task(int) doesn't have a default parameter because I found that if I set the same order value to every task, the final order ends up being kind of random.

@codecov

This comment has been minimized.

codecov bot commented Jun 23, 2018

Codecov Report

Merging #827 into master will increase coverage by 0.44%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #827      +/-   ##
==========================================
+ Coverage    66.1%   66.55%   +0.44%     
==========================================
  Files          14       14              
  Lines        1422     1438      +16     
  Branches      224      226       +2     
==========================================
+ Hits          940      957      +17     
+ Misses        431      430       -1     
  Partials       51       51
Impacted Files Coverage Δ
locust/core.py 84.25% <100%> (+1.75%) ⬆️

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 61b43c7...1c5a744. Read the comment docs.

@cgoldberg

This comment has been minimized.

Member

cgoldberg commented Jun 23, 2018

can you also include an update to the docs in your PR if we decide to merge it?

@Ramshell

This comment has been minimized.

Contributor

Ramshell commented Jun 24, 2018

@cgoldberg Yeah, Is it okay if I add and example in quick start and an example with explanation in writing a locust file?

@Ramshell

This comment has been minimized.

Contributor

Ramshell commented Jun 24, 2018

Well, finally I decided not to change the quick start because it's supposed to be short. Instead I've updated the api.rst and writing-locustfile.rst files. If there is anything else needed, just tell me :)

@ityoung

This comment has been minimized.

ityoung commented Jun 25, 2018

That's a real good work! I try to do that but met some trouble, thanks for your work!

@cgoldberg

This comment has been minimized.

Member

cgoldberg commented Jun 26, 2018

I'm +1 on merging this. Anyone else?
/cc @locustio/committers

@ityoung

This comment has been minimized.

ityoung commented Jun 26, 2018

I think you should add TaskSequence and seq_task to file locust/__init__.py, so that we can use from locust import seq_task.

@Ramshell

This comment has been minimized.

Contributor

Ramshell commented Jun 26, 2018

Adding TaskSequence and seq_task to file locust/init.py sounds like something useful. @cgoldberg what do you say?

@cgoldberg

This comment has been minimized.

Member

cgoldberg commented Jun 26, 2018

agreed.

@aldenpeterson-wf

This comment has been minimized.

Contributor

aldenpeterson-wf commented Jun 26, 2018

This LGTM

@Ramshell

This comment has been minimized.

Contributor

Ramshell commented Jun 26, 2018

If there is anything else you think I could add, just tell me! :)

@cgoldberg cgoldberg self-requested a review Jun 26, 2018

@cgoldberg

👍

@cgoldberg

This comment has been minimized.

Member

cgoldberg commented Jun 26, 2018

let's leave this for another day or 2 to see if any other maintainers comment, then I'll merge it.

@mbeacom

LGTM 🚢

@cgoldberg

This comment has been minimized.

Member

cgoldberg commented Jun 28, 2018

looks like consensus for this PR is a 👍 ... merging now.

@cgoldberg cgoldberg merged commit 30275af into locustio:master Jun 28, 2018

3 checks passed

codecov/patch 100% of diff hit (target 66.1%)
Details
codecov/project 66.55% (+0.44%) compared to 61b43c7
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@cgoldberg

This comment has been minimized.

Member

cgoldberg commented Jun 28, 2018

thanks very much for the contribution!

@Ramshell Ramshell deleted the Ramshell:feature/tasks-sequence-support branch Jun 28, 2018

@Ramshell

This comment has been minimized.

Contributor

Ramshell commented Jun 28, 2018

My pleasure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment