objective-php / rfc

ObjectivePHP RFCs repository
0 stars 1 forks source link

Parameter config directive #4

Open rflavien opened 6 years ago

rflavien commented 6 years ago

ObjectivePHP RFC: Parameter config directive

  • Version: 1.1.0
  • Date: 2018-02-22
  • Author: Flavien Rodrigues, rodrigues.flavien@gmail.com
  • Status: UNDER DISCUSSION

Summary

An app’s config is everything that is likely to vary between deploys (staging, production, developer environments, etc). I suggest, according to 12 factors app, a strict separation of config from code. Config varies substantially across deploys, code does not.

Currently we are able to override the config with a .local.php file. That's a way we can keep.

I suggest to add a feature to the config component, to reach the environment variables.

Proposal

The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in RFC 2119.

Using a Environment Directive with default value :

<?php

return [
    new EnvironmentDirective('APP_MODE', 'development');
];

Using an optional Environment Directive :

<?php

return [
    (new EnvironmentDirective('database.host'))->setOptional();
];

$config->get('database.host') instanceof UndefinedDirective; // true if no key `database.host` is not found in the environment

We also need to add another loader ObjectivePHP\Config\Loader\DotenvLoader. It will use vlucas/phpdotenv.

Backward Incompatible Changes

It will not include Incompatible Changes, it's a MINOR enhancement

Proposed ObjectivePHP Version(s)

objective-php/config 1.1.0

RFC Impact

To Components

NO

To Packages

NO

To Workflow

NO

To Configuration

It add this new way to manage changes between environments.

Future Scope

Future scope can be the extension of this directive with SecretParameterDirective that store encrypted value of the parameter.

Proposed Voting Choices

50%+1 majority

Links to external references, discussions, issues or RFCs

bcerati commented 6 years ago

That'd be nice to be able to configure easily an Objective application with environment variables.

I suggest to rename ParameterDirective to EnvParam or something like that.

I have a question though, how in your application do you retrieve this kind of config ?

Currently we get them like that: $config->subset(Param::class)->get('APP_MODE')

So for the environments it'll be $config->subset(ParameterDirective::class)->get('APP_MODE')

It will be confusing for the developers. Should i get "Param" or ParameterDirective ?

rflavien commented 6 years ago

I think it would be better to get things working like :

$config->subset(ApplicationConfiguration::class)->get('APP_MODE');

(Param is not an objective-php/config class and so is not ApplicationConfiguration)

fanshan commented 6 years ago

I think parameters must be mandatory by default.

If no default value are provided, it could be possible to make the directive optional :

(new ParameterDirective('database.host'))->optional();

new ParameterDirective('database.host', null) would not be an equivalent.

This is mean if database.host env variable is not defined then the directive database.host will not exist within the application.

Right ?

rflavien commented 6 years ago

@fanshan I agree that it's better to make it mandatory by default with the ->optional() to make it not mandatory.

If database.host is not defined in the env and the directive is not set as optional then it must throw the MissingMandatoryParameter

If database.host is not defined in the env and the directive is set as optional then we must provide a way to be aware of it. It can be something like :

$config->subset(DatabaseConfiguration::class)->get('host') instanceof UndefinedParameter; // this is true in this second case
rflavien commented 6 years ago

I added the ability to load variables from dotenv files.