beberlei / metrics

Simple library that abstracts different metrics collectors. I find this necessary to have a consistent and simple metrics (functional) API that doesn't cause vendor lock-in.
316 stars 38 forks source link

Add InfluxDB adapter #35

Closed davidfuhr closed 9 years ago

davidfuhr commented 9 years ago

Do you really want to support php 5.3?

lyrixx commented 9 years ago

Do you really want to support php 5.3?

Do you need to use a feature that exist only in php 5.4+ ?

But anyway, I'm fine with bumping to 5.4+

Finally, Could you add update the symfony2 bundle? If no, don't worry, I can take care of it.

davidfuhr commented 9 years ago

It's the library I use for InfluxDB (https://github.com/corley/influxdb-php-sdk) that requires PHP 5.4. Didn't look into it why it requires 5.4 exactly.

Would be fantastic if you could add the bundle part!

lyrixx commented 9 years ago

It's the library I use for InfluxDB (https://github.com/corley/influxdb-php-sdk) that requires PHP 5.4. Didn't look into it why it requires 5.4 exactly.

Ok, So there is no problem to keep this lib with php 5.3. Composer will manager that for us. You just have to skip your tests on php 5.3 (there is an anotation for that;))

Would be fantastic if you could add the bundle part!

Ok, Will do that in another PR.

davidfuhr commented 9 years ago

Ok, So there is no problem to keep this lib with php 5.3. Composer will manager that for us. You just have to skip your tests on php 5.3 (there is an anotation for that;))

That doesn't work in this case. I tried to add --ignore-platform-reqs to the composer install command, but unfortunately the library adds some autoload file to composer which causes phpunit to fail right away.

So i see three possible solutions:

  1. Remove php 5.3 from travis (this is what i did in my last commit)
  2. Use another Influx-Library (e.g. https://github.com/crodas/InfluxPHP)
  3. Split the Influx stuff to an extra library, but that would require adjustments to the factory to allow other collectors to be registered
lyrixx commented 9 years ago

Remove php 5.3 from travis (this is what i did in my last commit)

:+1: ;)

davidfuhr commented 9 years ago

Could you revert this change ?

Done. :)

lyrixx commented 9 years ago

Thanks David for working on this feature, this is much appreciated.