deployer icon indicating copy to clipboard operation
deployer copied to clipboard

Throw error on setting not serializable config value

Open rutgerrademaker opened this issue 3 years ago • 3 comments

  • Deployer version: v7.0.0-rc.8
  • Deployment OS: Ubuntu 20.04

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

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar

rutgerrademaker avatar Jul 11 '22 19:07 rutgerrademaker

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 avatar Jul 11 '22 19:07 antonmedv

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

antonmedv avatar Jul 11 '22 19:07 antonmedv

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();
    }
);

rutgerrademaker avatar Jul 11 '22 20:07 rutgerrademaker