search

pjcalvo / locust-influxdb-listener

LocustIO base project with a custom influxDB listener.
MIT License
26 stars 20 forks source link

fixed succes error + renamed tags to extra tags + make db creation att… #20

Closed pjcalvo closed 1 year ago

pjcalvo commented 1 year ago

This PR tackles 3 issues:

1) Renamed tags for additional to make a separation of what we internally use vs what the users is sending us 2) Fixed a problem where if response: was never resolving and therefore all requests where success. 3) Attempt to connect and create a influxdb every time instead of using a parameter for it.

If this PR is approved it will become our first 1.0 release...

NOTE: There are breaking changes on the names of the host and port parameters.

pjcalvo commented 1 year ago

@jmfiola Hey my friend. I have invited you to the repository, so you can also merge the PRs for me.

On this PR I changed the tags property that you added for additional_tags so that we can make a split between what we manage internally and what a customer sends us. If you look at this screenshot you can see that it still works.

image

Also, I made an additional changes to fix a bug and improve the database creation. I know that I should have done separates PRs apologies for that.

Finally, this will be our 1.0 release since it is stable and seems to have proven that it works.

jmfiola commented 1 year ago

Sorry for just now seeing this! I need to update my e-mail notifications for github I think.

jmfiola commented 1 year ago

Oh also you may want to bump the version to 1.0

pjcalvo commented 1 year ago

I am going to ads both params for now :) no problem

On Tue, 26 Sep 2023 at 00:11 Jacob Fiola @.***> wrote:

@.**** requested changes on this pull request.

In README.md https://github.com/pjcalvo/locust-influxdb-listener/pull/20#discussion_r1336433654 :

@@ -32,15 +32,16 @@ def on_locust_init(environment, **_kwargs): """

this settings matches the given docker-compose file

influxDBSettings = InfluxDBSettings(

  • influx_host = 'localhost',
  • influx_port = '8086',
  • host = 'localhost',

I would prefer if we didn't change these, as we have some locust-grasshopper kwargs that are using influx_host and influx_port and passing it in like this:

influx_db_settings = InfluxDBSettings( **influx_configuration, database="locust", )

So if you made this change then we would have to go and change it in a lot of places :(

— Reply to this email directly, view it on GitHub https://github.com/pjcalvo/locust-influxdb-listener/pull/20#pullrequestreview-1643056682, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABXCVOFTNRNJMLBQIYICDDTX4H6SLANCNFSM6AAAAAA44NUKDI . You are receiving this because you authored the thread.Message ID: @.***>

pjcalvo commented 1 year ago

Oh also you may want to bump the version to 1.0

Yeah but that would come on the release commit