laravel-self-diagnosis icon indicating copy to clipboard operation
laravel-self-diagnosis copied to clipboard

Remove schema prefix in ServersArePingable check

Open akalongman opened this issue 6 years ago • 3 comments

It would be very useful if there was an option per host for removing schema prefix (http://, https://), because when I want to use hostname from the config, like config('app.url'), it's very inconvenient to remove prefixes for each host. Something like:

\BeyondCode\SelfDiagnosis\Checks\ServersArePingable::class => [
    'servers' => [
        [
            'host'    => config('app.url'),
            'port'    => 80,
            'timeout' => 1,
            'remove_schema' => true,
        ],
    ]
]

@mpociot If you agree with the idea, I can implement and send PR

akalongman avatar Jan 14 '19 12:01 akalongman

Or maybe just allow strings like http://myhost.com:8080 and get host/port from there?

akalongman avatar Jan 14 '19 12:01 akalongman

Why don't you use something like parse_url(config('app.url'), PHP_URL_HOST)? Of course we could add further options, but it seems like a solid solution to me to only pass the actual host.

If we went for your solution, I would implement it properly and offer

\BeyondCode\SelfDiagnosis\Checks\ServersArePingable::class => [
    'servers' => [
        [
            'url' => 'https://some.host.com:2468',
            'timeout' => 1,
        ],
    ]
]

as alternative for

\BeyondCode\SelfDiagnosis\Checks\ServersArePingable::class => [
    'servers' => [
        [
            'host'    => 'some.host.com',
            'port'    => 2468,
            'timeout' => 1,
        ],
    ]
]

Namoshek avatar Jan 14 '19 17:01 Namoshek

If we went for your solution, I would implement it properly and offer

It's same what I suggested in the previous comment

Or maybe just allow strings like http://myhost.com:8080 and get host/port from there?

And I like this solution

akalongman avatar Jan 15 '19 09:01 akalongman