Hi @grifx ! Thanks for your pr!
I will check it asap, can you write here a way to reproduce the issue so I can do a couple of tests against it ?

Thanks again!

Hi @Algatux,

.env

DATABASE__INFLUXDB__UDP_PORT="8086"

app.php

use Symfony\Component\Dotenv\Dotenv;

$dotenv = new Dotenv();
$dotenv->load(__DIR__.'/.env');

config.yml

influx_db:
    (...)
        udp_port:  '%env(int:DATABASE__INFLUXDB__UDP_PORT)%'

The symfony config component doesn't get an integer as value for udp_port as one may think but a string that is resolved as an integer at runtime.

Thanks for your work.

Cheers,

@grifx, thx for these info, I will try asap!

@grifx I think that we need to add a validation before creating the Database Instance using ports as 'strings'.

\Algatux\InfluxDbBundle\Services\ConnectionFactory::createConnection uses the config to initialize a new \InfluxDB\Database but is typehinted, this can lead to type errors if you are not casting the env var as int

@Algatux You are right "this can lead to type errors if you are not casting the env var as int".

I should have made it clear that this PR is a temporary fix.
The root of the problem is being solved at the Configuration component level: symfony/symfony#23888

What we can do is to remove the typing in ConnectionFactory::createConnection and throw a verbose exception if the ports provided aren't integers.

@grifx Maybe a better solution can be a validation placed inside InfluxDbExtension::createConnection where we can simpli check that ports are numeric values and then cast them, otherwise an exception is thrown ... something like that IMHO

@Algatux Happy to do this. I thought you would be against it since a decision was made to enforce the types in the current implementation.

@grifx I would like only to mantain the type hints on the methods

@grifx any new about this pr ?

The options are booleans and integers. There is strictly nothing to fix here.

We are not responsible of the dotenv integration, Symfony is. This was "fixed" in Symfony 4.1. So you have only two solution: Upgrade to 4.1 or keep on parameters.yaml method waiting the upgrade,

@Algatux This PR should not be accepted.

@soullivaneuh didn't know that the problem was fixed. I'm closing this pr, there is no more need for a temporary workaround. Thanks

@Algatux It is, but not for Symfony 3.4. I don't know why they did this like this, but is not our responsibility to fix an issue from a new feature not correctly setup on a vendor, IMHO. ;-)

Sorry, I forgot about this PR.

Options are booleans and integers but this is a Symfony Bundle. It's the responsibility of the bundle to make the setup process as smooth as possible for members of this ecosystem.

It doesn't seem a lot of people are complaining so there is probably no need for it.