[WebProfilerBundle] Added feedback about the current symfony version #13626

wouterj · 2015-02-09T00:32:01Z

Description

This PR adds some visual and textual information about the Symfony version that is currently used. It'll indicate when the used version has reached eom or eol. I hope this will make people more aware of the fact that they should update (as I've seen quite some people using completely outdated Symfony versions).

Screenshot

PR Information

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes (didn't test though)
Fixed tickets	-
License	MIT
Doc PR	-

ghost · 2015-02-09T00:38:07Z

why not show the text right under "Symfony Documentation" if the version of EOM/EOL ?

javiereguiluz · 2015-02-09T07:24:12Z

Does this change imply to make one HTTP request to symfony.com for each page load in the dev environment (including AJAX page loads)?

stloyd · 2015-02-09T07:56:33Z

👎 for calling external website to get such info, even if that would be only on one page (not like now on every page with profiler).

IMO better would be to store such date in Kernel class, same like version consts, and then compare those in template...

class Kernel
{
    const EOL = '01-06-2015'; // or int 1433109600
}

merk · 2015-02-09T07:59:50Z

It could be implemented as an XHR request and use localStorage to cache the information?

javiereguiluz · 2015-02-09T08:29:27Z

I think we should not provide this feature. All the information is available at http://symfony.com/roadmap and developers can subscribe to receive emails about important end-of-life announcements.

However, if you insist in adding this feature, I'd add a Check version link in the extended information panel. When the user clicks on that link, an AJX request is made to symfony.com to get the information and display it to the user. For example:

ghost · 2015-02-09T08:45:44Z

@javiereguiluz : if the developers were keeping track, then they probably wouldn't be running such EOL versions. I don't think your solution solves the problem

ghost · 2015-02-09T08:47:13Z

why not just package up the output of the roadmap json and put it in sensiodistributionbundle? and then just parse that?

fabpot · 2015-02-09T08:50:06Z

I think having such a built-in feature is a great idea, but making one additional HTTP request to symfony.com for every page in the dev env of all Symfony apps is probably a bit too much.

@javiereguiluz IIUC, the goal is to automatically warn the developer, adding a link is not going to help at all.

@jrobeson An online version is always more flexible as it can take into account last-minute decisions like extending support for one version (because of any external factors).

linaori · 2015-02-09T08:51:16Z

I actually like this feature, I often see people come by and ask questions on IRC regarding symfony versions that aren't supported anymore. I think this feature should be implemented, but the information should be retrieved when building the container.

ghost · 2015-02-09T08:51:38Z

sure an online version is more flexible, but as they get a new version of the bundle, then it will include the updated roadmap. i think we shouldn't let the perfect be the enemy of the good here.

ghost · 2015-02-09T08:53:14Z

@iltar: i have a problem with symfony requiring internet access unless i explicitly told it to. like on composer updates

wouterj · 2015-02-09T08:53:18Z

The problem with an AJAX request is that'll be catched by the ajax data collector.

I agree that having a request every request isn't very great. We could cache it, so the request will only be executed whenever the cache was cleared?

linaori · 2015-02-09T08:56:20Z

@jrobeson in that case, you could turn it off and it would return the gray option: "Unable to retrieve information about the Symfony version.".

Another option could be to show a more specific message: "Unable to retrieve information about the Symfony version, enable to framework.version_check in your configuration".

fabpot · 2015-02-09T09:02:54Z

FWIW, I'm strongly against adding a configuration setting for that.

javiereguiluz · 2015-02-09T09:20:30Z

I wonder if this is really a "problem" we need to solve:

If you care about these things, you are already subscribed to the Symfony notifications, you execute the security: check command regularly, you have a continuous integration server that makes a lot of checks, etc.
If you don't care about these things, no matter how many times the framework tells you that the version is outdated. You won't update, because you don't care.

merk · 2015-02-09T09:24:29Z

@javiereguiluz The issue cant be classified under such black and white terms. It is true most developers who care will be subscribed to notifications, it isnt true that those who are not subscribed do not care and will not update.

There is an opportunity to educate here. A visual notification to indicate the status of symfony has an overall benefit to DX.

Even if it was hard-coded This version's support expires "roughly" 05/2015 in the Kernel it is hardly a terrible suggestion.

merk · 2015-02-09T09:25:40Z

I for one was not aware of a "security: check" command. Not everyone has extensive CI either.

jakzal · 2015-02-09T10:39:07Z

src/Symfony/Component/HttpKernel/DataCollector/ConfigDataCollector.php

     */
-    public function getAppName()
+    public function getappname()


Looks like something went wrong here (and below).

javiereguiluz · 2015-02-11T12:19:17Z

Just to recap the discussion so far, here are some of my arguments against this feature:

Detecting an outdated Symfony version is mostly useful in the production server, where you cannot use this feature.
It will slightly hurt application performance because of the added HTTP request.
It could severely hit symfony.com performance with a flood of requests.
There are other (better?) alternatives to get informed when Symfony is out-of date (like the Symfony Roadmap email notifications).
No one has ever asked for this feature. In the repository we have hundreds of specific feature requests, but not this one.
Anything related to software updates (detecting new versions, notifying about them, installing them, etc.) is one of the most annoying "features" of applications such as operating systems. If you don't believe me, ask around what do people think about the following "features":

wouterj · 2015-02-11T12:46:36Z

Thabks for your very opiniated recamp Javier. I think calling it your opinion would be much better.

jakzal · 2015-02-11T13:09:18Z

I'm 👍 on this, as long as the requests are cached (on both application and symfony.com). Could it be generated as part of bootstrap?

Comparing this feature to installers is not fair in my opinion. This is just an indicator, it doesn't force you to do anything. It does nothing but sits there (any annoyance coming from that is a good thing).

javiereguiluz · 2015-02-11T13:26:57Z

@wouterj you are right, this is my opinion. That's why the first phrase of my last comment said "... here are some of my arguments against this feature:"

I have yet another reason to not implement this feature:

Privacy: we are making unsolicited requests to a third-party host (symfony.com) without asking permission to the user. This could reveal sensitive information such as the IP address (and therefore, location) of the user.

DavidBadura · 2015-02-17T10:58:33Z

what about a console command like check:version? everyone can decide for themselves when and whether it should be checked.

wouterj · 2015-02-17T11:12:56Z

In that case, I vote for closing this PR. As executing a command is as simple as navigating to http://symfony.com/roadmap The issue is that many users that use older versions do that because they don't check these tools.

fabpot · 2015-02-17T11:24:00Z

@wouterj I do like this PR and the fact that it tells you that you have a problem without having to do anything.

wouterj · 2015-02-17T11:27:12Z

@fabpot I know that, I just responded to @DavidBadura (but forgot the mention...).

wouterj · 2015-04-03T13:26:14Z

I've disabled caching in the tests, as it was causing problems. Maybe we should disable requesting the version completely, to not flood symfony.com with requests? /cc @fabpot

wouterj · 2015-04-03T14:29:23Z

Test failures seem unrelated

fabpot · 2015-04-03T14:36:43Z

src/Symfony/Component/HttpKernel/DataCollector/ConfigDataCollector.php

+        if (file_exists($versionCachePath)) {
+            $versionInfo = json_decode(file_get_contents($versionCachePath), true);
+        } else {
+            $versionResponse = @file_get_contents('http://symfony.com/roadmap.json?version='.preg_replace('/-\w+\d*$/', '', $this->data['symfony_version']));


You should pass a version like x.y. A x.y.z will be anyway redirected to x.y. That would reduce the number of requests to symfony.com.

fabpot · 2015-04-03T14:45:05Z

@wouterj No need to disable for the tests, we have some cache on this resource. After fixing my comment, I'm 👍

wouterj · 2015-04-03T14:46:33Z

Ok, fine by me. I've fixed your comment (and pushed).

wouterj · 2015-04-03T14:48:44Z

Pushed one final safety guard: If the version was wrong for some reason and the error page (http://symfony.com/roadmap.json?version=2.4.x) was returned, it'll also show the "Unable to retrieve information" message.

fabpot · 2015-04-03T14:49:52Z

Thank you @wouterj.

…ymfony version (WouterJ) This PR was merged into the 2.7 branch. Discussion ---------- [WebProfilerBundle] Added feedback about the current symfony version Description --- This PR adds some visual and textual information about the Symfony version that is currently used. It'll indicate when the used version has reached eom or eol. I hope this will make people more aware of the fact that they should update (as I've seen quite some people using completely outdated Symfony versions). Screenshot --- ![sf-toolbar-version-info](https://cloud.githubusercontent.com/assets/749025/6099512/da59c99e-affa-11e4-8093-173857901769.png) PR Information --- | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes (didn't test though) | Fixed tickets | - | License | MIT | Doc PR | - Commits ------- a4551f9 Added feedback about the current symfony version

sprain · 2015-04-03T16:09:20Z

I'm a bit late to the party, but an option to turn this non-vital feature off per installation would be nice. As seen in the comments above, some users don't like it for privacy reasons or other concerns. Why not give them the option to do so?

shoomyth · 2015-04-03T18:18:05Z

This must be disableable

King2500 · 2015-04-06T14:29:35Z

@fabpot Can you explain why you are against adding a configuration setting for that?

javiereguiluz · 2015-04-06T14:36:00Z

One additional issue with this feature is that it's only available for 2.7. The irony is that 2.7 version is supported until May 2018. This means that your application will make thousands of HTTP requests during the next years to tell you that 2.7 version is still supported 😢

wouterj · 2015-04-06T14:39:26Z

please, @javiereguiluz, can you read the discussion and code? Your application will make just 1 request and 1 again on 31 May 2018.. Nice hu?!

The rest, I'm waiting on feedback from the core guys here. As @fabpot made it very clear that he doesn't like a config option for this, I'm not putting effort in creating a PR yet.

slider · 2015-04-14T11:23:51Z

@fabpot Are requests to http://symfony.com/roadmap.json stored/analyzed in some way on the serverside?

derrabus · 2015-04-14T11:35:25Z

I missed the whole discussion on this feature and just became aware of it because of the bug report I've just filed (#14349).

Please reconsider this feature, for all the reasons @javiereguiluz listed in this discussion. It's a functionality I would never have asked for. I want to be able to work in environments without permanent internet connection and I really don't want libraries to call home.

hhamon · 2015-04-14T11:38:40Z

Can't we have the end of life / end of maintainance dates in the composer.json file of Symfony? This way it's updated when running composer update command. As Composer will always have to go on the Internet, this may be the right moment to update those info and cache them somewhere (cache directory or composer.lock file).

derrabus · 2015-04-14T12:01:00Z

The roadmap of Symfony is pretty static. We already know EOM and EOL dates of the 2.7 branch, so we could simply add this information as constants to the Symfony\Component\HttpKernel\Kernel class, right below the other version constants.

If those dates change in the future, we would update them and ship them with the next release. I don't expect the maintenance period to be reduced afterwards, so the worst thing that might happen would be that the guy who is running 2.7.16 although 2.7.42 is the most current one gets a notification that his version is out of support, although the maintenance period has been extended with the 2.7.21 release. But he's running an old version anyway, so I could live with that.

apfelbox · 2015-04-22T06:48:32Z

I very much like and support the reasoning from @derrabus and would love to see it implemented. Although I probably would include it in a separate composer package. This would work the same way as @derrabus explained it for the constants in the kernel – as in that the user gets a notification of his old version, regardless whether something has changed in the roadmap or not. So (s)he would issue a composer update which would automatically fetch the new version of the roadmap package (with the new version data).

This library would mainly extract the logic and data from http://symfony.com/roadmap into its own package.

This package could optionally even include the dynamic (and cached) call to a web API, which would just override the locally (inside the package) stored default version data.

…rsion feedback (derrabus) This PR was squashed before being merged into the 2.7 branch (closes #14351). Discussion ---------- [WebProfilerBundle] [HttpKernel] A static approach to version feedback | Q | A | ------------- | --- | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #14349, #13626 | License | MIT | Doc PR | N/A This is an alternative approach for the version feedback feature of PR #13626. The original PR performed a server-side HTTP request to symfony.com, which might be problematic in certain environments, see #14349. This PR makes the following assumptions: * Symfony's release roadmap is rarely changed * After the first release of a branch, the Symfony development team is committed to the announced support dates. * The support period of a stable branch might be extended, but it's usually not reduced. Given those assumptions, the EOM and EOL dates may be shipped with the Symfony release and may be updated with a later bugfix release. The information would be available offline without any need to query Symfony's servers. If the user is running the latest bugfix release of a branch, the EOM/EOL dates should be accurate. If he's running an earlier version, he might get a false positive warning about reaching EOM or EOL if the support period has been extended in the meantime. But since he's running an outdated version anyway, would this really be a problem? Commits ------- f5f3bba [WebProfilerBundle] [HttpKernel] A static approach to version feedback

… (GromNaN) This PR was merged into the 2.7 branch. Discussion ---------- [WebProfilerBundle] Fix compatiblity with HttpKernel < 2.7 The WebProfilerBundle version 2.7 is supposed to be compatible with HttpKernel 2.4, but the template of the config profile uses a property `symfonyState` that has been added in Symfony 2.7 by #13626. > Twig_Error_Runtime: Method "symfonyState" for object "Symfony\Component\HttpKernel\DataCollector\ConfigDataCollector" does not exist in "@WebProfiler/Collector/config.html.twig" at line 12 This fix makes the property optional. | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | silexphp/Silex-WebProfiler#63 | License | MIT Commits ------- 700fcf1 Fix WebProfilerBundle compatiblity with HttpKernel < 2.7

…rrent symfony version (WouterJ) This PR was merged into the 2.7 branch. Discussion ---------- [WebProfilerBundle] Added feedback about the current symfony version Description --- This PR adds some visual and textual information about the Symfony version that is currently used. It'll indicate when the used version has reached eom or eol. I hope this will make people more aware of the fact that they should update (as I've seen quite some people using completely outdated Symfony versions). Screenshot --- ![sf-toolbar-version-info](https://cloud.githubusercontent.com/assets/749025/6099512/da59c99e-affa-11e4-8093-173857901769.png) PR Information --- | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes (didn't test though) | Fixed tickets | - | License | MIT | Doc PR | - Commits ------- a4551f9 Added feedback about the current symfony version

wouterj force-pushed the sf-version-info branch 2 times, most recently from e518aee to d3a872b Compare February 9, 2015 00:36

jakzal reviewed Feb 9, 2015
View reviewed changes

wouterj force-pushed the sf-version-info branch from a7782cd to d3e8e63 Compare April 3, 2015 13:25

fabpot reviewed Apr 3, 2015
View reviewed changes

wouterj force-pushed the sf-version-info branch from d3e8e63 to cb0a62c Compare April 3, 2015 14:45

Added feedback about the current symfony version

a4551f9

wouterj force-pushed the sf-version-info branch from cb0a62c to a4551f9 Compare April 3, 2015 14:48

fabpot merged commit a4551f9 into symfony:2.7 Apr 3, 2015

wouterj deleted the sf-version-info branch April 3, 2015 15:13

derrabus mentioned this pull request Apr 14, 2015

[WebProfilerBundle] [HttpKernel] Version feedback feature slows down application behind corporate proxy #14349

Closed

derrabus mentioned this pull request Apr 14, 2015

[WebProfilerBundle] [HttpKernel] A static approach to version feedback #14351

Closed

GromNaN mentioned this pull request May 15, 2015

[WebProfilerBundle] Fix compatiblity with HttpKernel < 2.7 #14645

Merged

Uh oh!

[WebProfilerBundle] Added feedback about the current symfony version #13626

[WebProfilerBundle] Added feedback about the current symfony version #13626

Uh oh!

Conversation

wouterj commented Feb 9, 2015

Description

Screenshot

PR Information

Uh oh!

ghost commented Feb 9, 2015

Uh oh!

javiereguiluz commented Feb 9, 2015

Uh oh!

stloyd commented Feb 9, 2015

Uh oh!

merk commented Feb 9, 2015

Uh oh!

javiereguiluz commented Feb 9, 2015

Uh oh!

ghost commented Feb 9, 2015

Uh oh!

ghost commented Feb 9, 2015

Uh oh!

fabpot commented Feb 9, 2015

Uh oh!

linaori commented Feb 9, 2015

Uh oh!

ghost commented Feb 9, 2015

Uh oh!

ghost commented Feb 9, 2015

Uh oh!

wouterj commented Feb 9, 2015

Uh oh!

linaori commented Feb 9, 2015

Uh oh!

fabpot commented Feb 9, 2015

Uh oh!

javiereguiluz commented Feb 9, 2015

Uh oh!

merk commented Feb 9, 2015

Uh oh!

merk commented Feb 9, 2015

Uh oh!

jakzal Feb 9, 2015

Choose a reason for hiding this comment

Uh oh!

javiereguiluz commented Feb 11, 2015

Uh oh!

wouterj commented Feb 11, 2015

Uh oh!

jakzal commented Feb 11, 2015

Uh oh!

javiereguiluz commented Feb 11, 2015

Uh oh!

DavidBadura commented Feb 17, 2015

Uh oh!

wouterj commented Feb 17, 2015

Uh oh!

fabpot commented Feb 17, 2015

Uh oh!

wouterj commented Feb 17, 2015

Uh oh!

wouterj commented Apr 3, 2015

Uh oh!

wouterj commented Apr 3, 2015

Uh oh!

fabpot Apr 3, 2015

Choose a reason for hiding this comment

Uh oh!

fabpot commented Apr 3, 2015

Uh oh!

wouterj commented Apr 3, 2015

Uh oh!

wouterj commented Apr 3, 2015

Uh oh!

fabpot commented Apr 3, 2015

Uh oh!

sprain commented Apr 3, 2015

Uh oh!