Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FrameworkBundle] Debug container environment variables #27684

Merged
merged 1 commit into from Mar 25, 2019

Conversation

Projects
None yet
7 participants
@ro0NL
Copy link
Contributor

commented Jun 22, 2018

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #...
License MIT
Doc PR symfony/symfony-docs#...

This PR is an attempt to ease debugging environment variables used by the container. Main purpose is to tell which are missing and show detailed usage information.

image

image

@ro0NL ro0NL force-pushed the ro0NL:di/debug-env branch 2 times, most recently from 612f654 to 5467967 Jun 23, 2018

@ro0NL

This comment has been minimized.

Copy link
Contributor Author

commented Jun 23, 2018

screenshot updated and JSON version included. I think the data set is pretty nice, and leaves us figuring out how to display processors/processed value in the text table without cluttering output too much. Still need to extract the actual prefixes though :)

@ro0NL ro0NL force-pushed the ro0NL:di/debug-env branch from 5467967 to e00d2f1 Jun 25, 2018

@ro0NL

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2018

text version conceptually ready :)

@fabpot
Copy link
Member

left a comment

I really like this one and I would like to get it for 4.2. @ro0NL Can you finish it? Any comments @nicolas-grekas

@nicolas-grekas
Copy link
Member

left a comment

Actually, do we need the json/xml descriptors?
I'm not sure they're useful, but I'm sure they're costly to maintain.

@ro0NL

This comment has been minimized.

Copy link
Contributor Author

commented Oct 10, 2018

Actually, do we need the json/xml descriptors?

not really, do you suggest to throw from any format other then text?

@fabpot

This comment has been minimized.

Copy link
Member

commented Oct 10, 2018

Throwing an exception sounds good to me.

@ro0NL ro0NL referenced this pull request Oct 17, 2018

Merged

[Console] Add dumper #28898

@ro0NL

This comment has been minimized.

Copy link
Contributor Author

commented Oct 25, 2018

aiming for

  • debug:container --env-vars (shows name, default- and real value horizontally)
  • debug:container --env-vars APP_ (shows matching envs with prefix and processed value vertically)
  • throw in markdown/xml, support text/json (having at least 1 machine format sounds reasonable and it's already there :))
@ro0NL

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2018

@fabpot for 4.2? Do you suggest to keep the VarDumper dependency for now.. Form component already does it. Otherwise we need to squeeze #28898 as well

@ro0NL ro0NL force-pushed the ro0NL:di/debug-env branch 2 times, most recently from 4e276b0 to e5467bb Oct 27, 2018

@ro0NL

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2018

screenshots updated

@ro0NL

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2018

i dropped JSON support as well, no real need for a machine format to inspect/debug envs. They are bound to the current environment/runtime anyway.

@ro0NL ro0NL force-pushed the ro0NL:di/debug-env branch from e5467bb to d42945f Oct 27, 2018

@xabbuh

This comment has been minimized.

Copy link
Member

commented Mar 8, 2019

@ro0NL Any chance to finish this one for 4.3? :)

fabpot added a commit that referenced this pull request Mar 24, 2019

feature #28898 [Console] Add dumper (ro0NL)
This PR was squashed before being merged into the 4.3-dev branch (closes #28898).

Discussion
----------

[Console] Add dumper

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #...   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#10502

This PR adds a new `Dumper` helper in the Console component. As there are 2 types of dumps

- debug purpose (e.g. `dd()`, `dump()`)
- output purpose (see #24208, #27684)

For the latter we cannot use the global system (debug) dumper, i.e. `VarDumper::dump()`, we need something tied to the current output and dependency free. Here it is:

```php
$io = new SymfonyStyle($input, $output);
$dumper = new Dumper($io);

$io->writeln($dumper([-0.5, 0, 1]));
$io->writeln($dumper(new \stdClass()));
$io->writeln($dumper(123));
$io->writeln($dumper('foo'));
$io->writeln($dumper(null));
$io->writeln($dumper(true));
```

With VarDumper comonent:

![image](https://user-images.githubusercontent.com/1047696/47069483-4cc26f80-d1ef-11e8-902e-2f9b0f040f25.png)

Without:

![image](https://user-images.githubusercontent.com/1047696/47069517-6663b700-d1ef-11e8-9328-ae1db0b83d7e.png)

> #27684 (comment) var-dumper is not a mandatory dep of fwb, can we do without?

Now we can  :)

Commits
-------

fc7465c [Console] Add dumper

@ro0NL ro0NL force-pushed the ro0NL:di/debug-env branch from 1e20b16 to 0326602 Mar 24, 2019

@ro0NL ro0NL force-pushed the ro0NL:di/debug-env branch 4 times, most recently from bfa89ca to 6e24c8e Mar 24, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented Mar 24, 2019

@ro0NL What needs to be done before merging?

@ro0NL ro0NL force-pushed the ro0NL:di/debug-env branch 2 times, most recently from f457a5b to 9050566 Mar 24, 2019

@ro0NL

This comment has been minimized.

Copy link
Contributor Author

commented Mar 24, 2019

@fabpot ready. Failures unrelated.

@ro0NL ro0NL referenced this pull request Mar 25, 2019

Merged

[Form][Console] Use dumper #30666

@fabpot

fabpot approved these changes Mar 25, 2019

@fabpot fabpot force-pushed the ro0NL:di/debug-env branch from 9921d1e to b813a05 Mar 25, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented Mar 25, 2019

Thank you @ro0NL.

@fabpot fabpot merged commit b813a05 into symfony:master Mar 25, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Mar 25, 2019

feature #27684 [FrameworkBundle] Debug container environment variable…
…s (ro0NL)

This PR was squashed before being merged into the 4.3-dev branch (closes #27684).

Discussion
----------

[FrameworkBundle] Debug container environment variables

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #...   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

This PR is an attempt to ease debugging environment variables used by the container. Main purpose is to tell which are missing and show detailed usage information.

![image](https://user-images.githubusercontent.com/1047696/47601226-ae43c480-d9cd-11e8-926b-8c49069189fc.png)

![image](https://user-images.githubusercontent.com/1047696/47601234-d7645500-d9cd-11e8-9e3f-d1c2ad85dbc5.png)

Commits
-------

b813a05 [FrameworkBundle] Debug container environment variables

@ro0NL ro0NL deleted the ro0NL:di/debug-env branch Mar 25, 2019

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019

@fabpot fabpot referenced this pull request May 9, 2019

Merged

Release v4.3.0-BETA1 #31435

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.