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

locust fingerprints with all dependencies added #311

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

mr-mosi
Copy link
Contributor

@mr-mosi mr-mosi commented Apr 14, 2023

Hi dear tsunami team,

I just finished getting fingerprints for Locust and put them in the desired directory.
please check it. last pull request can be ignored. thanks for your time.

@tooryx tooryx added Contributor main The main issue a contributor is working on (top of the contribution queue). fingerprints labels Feb 1, 2024
@tooryx tooryx linked an issue Feb 1, 2024 that may be closed by this pull request
@tooryx tooryx self-requested a review August 7, 2024 14:12
@tooryx
Copy link
Member

tooryx commented Aug 7, 2024

Hi @mr-mosi,

It seems like our automation did not trigger on this PR.
Could you pull master on your branch and push the changes?

~tooryx

@mr-mosi
Copy link
Contributor Author

mr-mosi commented Dec 5, 2024

Hi @tooryx yes sure. I will push the changes.

@mr-mosi
Copy link
Contributor Author

mr-mosi commented Feb 8, 2025

@tooryx Could you remove the PRP:Inactive label?

@tooryx
Copy link
Member

tooryx commented Feb 10, 2025

Hey @mr-mosi,

It seems like the generated fingerprints are missing from the PR. Can you add them?

Thank you,
~tooryx

@mr-mosi
Copy link
Contributor Author

mr-mosi commented Feb 12, 2025

@tooryx I fixed some bugs, moved scripts to the correct directory and created the .binproto file

Add ending newline
Copy link

@savio-doyensec savio-doyensec left a comment

Choose a reason for hiding this comment

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

Hey @mr-mosi, thanks for your contribution!

I've left some comments on a few things that can be improved.

Please also ensure to do the following:

  • Sync the fork with the main repo
  • Delete the old files in community/fingerprinters
  • The binproto file is in the wrong directory, it should be in google/fingerprinters/web/src/main/resources/fingerprinters/web/data/community/locust.binproto (notice it's community/locust.binproto, not google/locust.binproto at the end)
  • Add the missing versions to the versions.txt file. I see that there are images available from 0.12.1 up to 2.33.0. I've made sure the changes I suggested work on all these versions.

@mr-mosi
Copy link
Contributor Author

mr-mosi commented Feb 26, 2025

@savio-doyensec
Copy link

@savio-doyensec Hi, do you want to add versions that end containing ".dev" like this one too: https://hub.docker.com/layers/locustio/locust/2.32.4.dev25/images/sha256-66f62cc0665b93845026f9a4068a00dad674170a4d48cdc2a7d320747f37b016 ?

I think having only the stable versions is good enough. I launched a few different versions and noticed that the interface rarely changes anyway.

@mr-mosi
Copy link
Contributor Author

mr-mosi commented Feb 26, 2025

I'll update the .bintproto file soon.

@mr-mosi
Copy link
Contributor Author

mr-mosi commented Feb 26, 2025

I've also added the getAllTags.py IDK if this is useful to keep it or not. it is a very simple script generated by AI however.

@savio-doyensec
Copy link

I've also added the getAllTags.py IDK if this is useful to keep it or not. it is a very simple script generated by AI however.

Since this is not required for the updater I think it's better to remove it to keep the repository clean, thanks!

Copy link

@savio-doyensec savio-doyensec left a comment

Choose a reason for hiding this comment

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

Ok, here are a few more minor things to fix and then the plugin is good to go.

Please also remember to submit an updated binproto file with the new versions after applying the changes

…se config and locustfile to comply with versions <=1.14.5
@mr-mosi
Copy link
Contributor Author

mr-mosi commented Feb 27, 2025

Hi @savio-doyensec
The locustfile.py example is not compatible with some versions including the
2.26.0
2.25.0
So I had to change the example python file.

Also, for versions below 1.14.5 the docker-compose and locust python file is changed too

@savio-doyensec
Copy link

LGTM - Approved
@maoning we can merge this.

Reviewer: Savio, Doyensec
Plugin: Locust Fingerprinter
Drawbacks: None

@lokiuox lokiuox added the lgtm label Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contributor main The main issue a contributor is working on (top of the contribution queue). fingerprints lgtm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request for update Locust fingerprint
4 participants