deployphp / deployer

The PHP deployment tool with support for popular frameworks out of the box
https://deployer.org
MIT License
10.56k stars 1.48k forks source link

Throw error on setting not serializable config value #3192

Closed rutgerrademaker closed 1 week ago

rutgerrademaker commented 2 years ago

When setting a config value to be a specific class, somewhere along the way the config value gets converted to an array I went down the rabbit hole (luckily it was a small rabbit) and think I found the root cause

In https://github.com/deployphp/deployer/blob/master/src/Configuration/Configuration.php#L213 we see

    public function load(): void
    {
        if (!$this->has('master_url')) {
            return;
        }

        // This part somehow transforms the object into an empty array
        $values = Httpie::get($this->get('master_url') . '/load')
            ->jsonBody([
                'host' => $this->get('alias'),
            ])
            ->getJson();

        $this->update($values);
    }

I do not quite get yet why Deployer needs to get content from http://localhost:45113/load here, but If it truly wants to do that I guess we can no longer store objects in the configuration. If that would be the case it would make sense to not even allow storing objects in the first place.

We could work around that, but maybe you see another solution? Could you maybe elaborate on (the future of) storing Objects in the config in combination with this?

  Please, provide a minimal reproducible example of deploy.php
<?php

namespace Deployer;

// % composer global show deployer/deployer
// Changed current directory to /home/rutger/.config/composer
// name     : deployer/deployer
// descrip. : Deployment Tool
// keywords :
// versions : * v7.0.0-rc.8
require_once getenv("HOME"). '/.config/composer/vendor/autoload.php';

class TestClass
{
    public static $test = 'test';

    public function execute(): string
    {
        return(get_class($this));
    }
}

host('localhost')->set('some_config', new TestClass());

print_r('Here the config returns a class' . PHP_EOL);
$someConfig = host('localhost')->get('some_config');
print_r($someConfig);

task('test', function () {
        $someConfig = host('localhost')->get('some_config');
        print_r('Here the config returns an array' . PHP_EOL);
        print_r($someConfig);
        echo $someConfig->execute();
    }
);

Upvote & Fund

Fund with Polar

antonmedv commented 2 years ago

Yes, Deployer can store only JSON seriazable objects.

This is done to be able to communicate between tasks, as each task runs in parallel on separate worker process. Can uses httpie to send state to the master.

antonmedv commented 2 years ago

I think better error message in this pace will be helpful. Maybe you can add such message in PR?

rutgerrademaker commented 2 years ago

Thanks again for the quick reply

The output of the example looks like

 % ~/.config/composer/vendor/deployer/deployer/bin/dep test localhost
Here the config returns a class
Deployer\TestClass Object
(
)
task test
Here the config returns a class
Deployer\TestClass Object
(
)
Here the config returns an array
Array
(
)
[localhost]  Error  in deploy.php on line 33:
[localhost]
[localhost]   Call to a member function execute() on array
[localhost]

In Deployer 6 though this worked like a charm If this stops working in Deployer 7 it would be worth not allowing to set an object in the first place AND mentioning this as breaking change for those coming from deployer 7

With your information, my suggested workaround for those facing the same issue would be

host('localhost')->set('some_config', get_class(new TestClass()));

task('test', function () {
        $someConfig = host('localhost')->get('some_config');
        $someConfigClass = new $someConfig;
        echo $someConfigClass->execute();
    }
);
github-actions[bot] commented 1 week ago

This issue has been automatically closed. Please, open a discussion for bug reports and feature requests.

Read more: [https://github.com/deployphp/deployer/discussions/3888]