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

Adding tagging based on request context #16

Merged
merged 6 commits into from
Aug 4, 2022
Merged

Conversation

jmfiola
Copy link
Collaborator

@jmfiola jmfiola commented Jul 18, 2022

if someone now does something like: locustClient.get('asdf.com/get', context={'environment':'prod'}), then that environment tag will be added for that request when it is reported to influxdb. This allows us to do more granular filtering once we want to query that influxdb data.

There were also some linting changes that my Pycharm decided to do. I can get rid of those if you want! Thanks.

@jmfiola
Copy link
Collaborator Author

jmfiola commented Jul 18, 2022

@pjcalvo FYI

@pjcalvo
Copy link
Owner

pjcalvo commented Jul 19, 2022

Will Merge and release tomorrow. Thanks for doing this.

Copy link
Owner

@pjcalvo pjcalvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the comments.

Also this MR includes a lot of small changes that I guess ur IDE suggested but are not related to the specific change you are trying to include. Please can u clean the PR a little bit.

Also, I assume that the increase of version is mantatory right?

example/requirements.txt Outdated Show resolved Hide resolved
locust_influxdb_listener/__init__.py Show resolved Hide resolved
@jmfiola
Copy link
Collaborator Author

jmfiola commented Jul 20, 2022

I reverted almost all the formatting changes that my IDE was suggesting. There's a few pesky ones that it really wants me to do though..

Yes, the increase in the locust version is mandatory due to us now using the request hook instead of request_success and request_failure

@jmfiola jmfiola requested a review from pjcalvo July 20, 2022 21:33
@pjcalvo
Copy link
Owner

pjcalvo commented Jul 21, 2022

Awesome, I will take another look. Would you mind adding the required version somewhere in the top of the README.md file? Thanks

@pjcalvo
Copy link
Owner

pjcalvo commented Jul 21, 2022

So I checked locust logs and indeed the on_failure and on_success events are deprecated and will be removed soon, but the requests hook has been out since version 1.5.0 and not on 2.8.0 like this PR is suggesting. I will check if we can go back to 1.5.0 with success and then leave that version.

I would hate to force users to use upgrade to 2.8.0 for a feature that has been out since previous versions.

https://github.com/locustio/locust/releases/tag/1.5.0

@jmfiola
Copy link
Collaborator Author

jmfiola commented Aug 1, 2022

@pjcalvo Changes have been made to versions and README as requested. Sorry this was a little late, I was on vacation!

example/README.md Outdated Show resolved Hide resolved
@pjcalvo
Copy link
Owner

pjcalvo commented Aug 4, 2022

Nice thanks. I was also on holiday. One more small change and we are ready to release this version.

Co-authored-by: Pablillo Calvo <pjcalvov@gmail.com>
@jmfiola
Copy link
Collaborator Author

jmfiola commented Aug 4, 2022

Nice thanks. I was also on holiday. One more small change and we are ready to release this version.

No worries! Change made. Thank you.

@pjcalvo pjcalvo merged commit eaf6681 into pjcalvo:main Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants