Edgewall Software
Modify

Opened 10 years ago

Closed 9 years ago

#426 closed enhancement (fixed)

Replace XML/SWF Charts with an open system

Reported by: dfraser Owned by: dfraser
Priority: major Milestone: 0.6
Component: Trac plugin Version: 0.6b2
Keywords: charts Cc: silk@…, tim@…, felix.schwarz@…, mpotter@…, hodgestar
Operating System:

Description (last modified by dfraser)

XML/SWF Charts:

  • has some layout problems on some machines (which are probably fixable)
  • is not open source/free software (which is probably not fixable)
  • only generates Flash charts (which is a bit icky)

This ticket is to collect the simplest ideas to replace it with something else. Sample options include (please add serious contenders here):

Attachments (7)

bitten-426-open-charting-1.patch (122.0 KB) - added by dfraser 10 years ago.
Initial reimplementation of charting in flot
bitten-charts.png (240.6 KB) - added by dfraser 10 years ago.
Screenshot (note that lint and coverage aren't enabled on these tests)
bitten-426-open-charting-2.patch (123.2 KB) - added by hodgestar 10 years ago.
Updated patched (use either json or simplejson, move legend, remove old charting templates, remove non-functional xaxis ticks).
bitten-xml-charts.png (34.6 KB) - added by hodgestar 10 years ago.
Screenshot of old XML charts for comparison.
bitten-jquery-charts.png (83.3 KB) - added by hodgestar 10 years ago.
Screenshot of new JQuery charts (made using patch 2).
bitten-426-open-charting-3.patch (159.1 KB) - added by dfraser 9 years ago.
Reworked patch to apply to latest trunk
resizable-charts.diff (1.9 KB) - added by hodgestar 9 years ago.
Add config option for chart style attribute (allows chart width and height to be configured).

Download all attachments as: .zip

Change History (48)

comment:1 follow-up: Changed 10 years ago by silk <silk@…>

  • Cc silk@… added

Just a comment, why I think Google Chart API would not be a good solution.

For closed source development, sending even the data for graphs to external service may be unacceptable (politicaly, etc). I know it would be a big problem for us.

comment:2 in reply to: ↑ 1 Changed 10 years ago by dfraser

Replying to silk <silk@…>:

Just a comment, why I think Google Chart API would not be a good solution.

For closed source development, sending even the data for graphs to external service may be unacceptable (politicaly, etc). I know it would be a big problem for us.

Absolutely, I just included it for reference as it was used by other Trac extensions.

comment:3 Changed 10 years ago by cboos

  • Type changed from task to enhancement

Definitely a must. I would suggest a jQuery based solution, like Flot.

comment:4 Changed 10 years ago by mgood

jqPlot is another nice jQuery-based library.

Changed 10 years ago by dfraser

Initial reimplementation of charting in flot

comment:5 Changed 10 years ago by dfraser

The attached patch includes the flot.js and excanvas.js files. It seems to work, but not been seriously tested. There's definitely room for work on visuals, but for me it's already nicer than the flash charts :-)

Changed 10 years ago by dfraser

Screenshot (note that lint and coverage aren't enabled on these tests)

comment:6 Changed 10 years ago by cmlenz

gRaphaël looks pretty darn nice, too.

comment:7 follow-up: Changed 10 years ago by Tim Niemueller <tim@…>

  • Cc tim@… added

I have packaged Bitten for Fedora and FreeBSD and I'm about to submit these packages for inclusion. For Fedora this is not possible with the given chart library, given that it is not open source and therefore not acceptable as per http://fedoraproject.org/wiki/Licensing.

We are not using charting feature in our project, yet. Hence I didn't test the supplied patch. Can someone confirm that the patch is working and a equal replacement?

In that case I'll include the open charting patch in the package and remove the existing stuff. Is there a chance to see this already in 0.6, given that there is a patch available that can do this already?

comment:8 in reply to: ↑ 7 ; follow-up: Changed 10 years ago by anonymous

Replying to Tim Niemueller <tim@…>:

I have packaged Bitten for Fedora and FreeBSD and I'm about to submit these packages for inclusion. For Fedora this is not possible with the given chart library, given that it is not open source and therefore not acceptable as per http://fedoraproject.org/wiki/Licensing.

That's great news...

We are not using charting feature in our project, yet. Hence I didn't test the supplied patch. Can someone confirm that the patch is working and a equal replacement?

The patch is working but is not yet an equal placement - the charts it generates are functional but fairly ugly.

In that case I'll include the open charting patch in the package and remove the existing stuff. Is there a chance to see this already in 0.6, given that there is a patch available that can do this already?

Given this was a first stab and 0.6 is a desparate-release-because-none-has-been-made for ages, it was meant to be deferred... you could bring up a discussion on IRC/the mailing list if you want

comment:9 in reply to: ↑ 8 Changed 10 years ago by dfraser

Replying to anonymous:

Replying to Tim Niemueller <tim@…>:

We are not using charting feature in our project, yet. Hence I didn't test the supplied patch. Can someone confirm that the patch is working and a equal replacement?

The patch is working but is not yet an equal placement - the charts it generates are functional but fairly ugly.

Forgot to login - that comment was from me (dfraser)

comment:10 follow-up: Changed 10 years ago by Tim Niemueller <tim@…>

Are you working on this atm? Is it feature-wise equali with the flash charts despite looking ugly? What has to be done otherwise? Would you say it is an acceptable compromise to use your patch and getting it packaged and included in Fedora? Otherwise that would have to wait for some time...

comment:11 in reply to: ↑ 10 Changed 10 years ago by dfraser

Replying to Tim Niemueller <tim@…>:

Are you working on this atm? Is it feature-wise equali with the flash charts despite looking ugly? What has to be done otherwise? Would you say it is an acceptable compromise to use your patch and getting it packaged and included in Fedora? Otherwise that would have to wait for some time...

I generated the initial patch, and since the plan was for it to be deferred to 0.7 I haven't put any more effort into it. But I am running a production server using it (on Fedora 7, I produce my own rpms) - the plan is for either this patch or an equivalent patch using an alternative graphing system to make it into bitten for the next milestone, so I think per Fedora's policies it would be best to include it (the only alternative being to remove charting). But I think you should test yourself and see since you're the one who's going to submit it.

Changed 10 years ago by hodgestar

Updated patched (use either json or simplejson, move legend, remove old charting templates, remove non-functional xaxis ticks).

Changed 10 years ago by hodgestar

Screenshot of old XML charts for comparison.

Changed 10 years ago by hodgestar

Screenshot of new JQuery charts (made using patch 2).

comment:12 Changed 10 years ago by Felix Schwarz <felix.schwarz@…>

  • Cc felix.schwarz@… added

comment:13 Changed 9 years ago by mpotter@…

  • Cc mpotter@… added

comment:14 Changed 9 years ago by hodgestar

  • Owner set to hodgestar
  • Status changed from new to assigned

comment:15 Changed 9 years ago by hodgestar

  • Cc hodgestar added

Changed 9 years ago by dfraser

Reworked patch to apply to latest trunk

comment:16 Changed 9 years ago by dfraser

The attached patch seems OK to me now - we could potentially do some stylistic adjustments, but as far as I'm concerned, this is at least ready for trunk

comment:17 Changed 9 years ago by dfraser

Committed in r846, but leaving this open for testing...

comment:18 Changed 9 years ago by dfraser

Backported in [846]; as of that point trunk and 0.6.x are identical except for versioning...

comment:19 Changed 9 years ago by anatoly techtonik <techtonik@…>

jQuery sparkline is another library to consider for inline charts http://omnipotent.net/jquery.sparkline/

comment:20 Changed 9 years ago by hodgestar

  • Milestone changed from 0.7 to 0.6
  • Version changed from dev to 0.6b2

comment:21 follow-up: Changed 9 years ago by dfraser

Discussion in mailing list on the added requirement for a JSON parser includes this:

Trac has a JSON encoder on trunk, which handles most simple cases. It is used to serialize e.g. ticket field data for the custom query page. See the bottom of (lines 282 and following):

http://trac.edgewall.org/browser/trunk/trac/util/presentation.py

The plan is to use that as a fallback for the inbuilt JSON library in Python 2.6

comment:22 in reply to: ↑ 21 Changed 9 years ago by dfraser

Replying to dfraser:

Discussion in mailing list on the added requirement for a JSON parser includes this:

Trac has a JSON encoder on trunk, which handles most simple cases. It is used to serialize e.g. ticket field data for the custom query page. See the bottom of (lines 282 and following):

http://trac.edgewall.org/browser/trunk/trac/util/presentation.py

The plan is to use that as a fallback for the inbuilt JSON library in Python 2.6

Unfortunately it's only available in Trac 0.12 onwards. Bleaugh

comment:23 Changed 9 years ago by dfraser

  • Owner changed from hodgestar to dfraser
  • Status changed from assigned to new

Committed a version that handles using trac's json method, or if not Python's, or if not, an internal version in [857], and backported it in [858]

comment:24 Changed 9 years ago by dfraser

  • Status changed from new to assigned

comment:25 Changed 9 years ago by dfraser

  • Description modified (diff)

comment:26 Changed 9 years ago by wbell

Are there any upgrade requirements here? I'm running an instance of Trac 0.11.5 and oddly enough having problems getting to the build pages (/build/config/id) even though the charts look good on the configuration pages. It's not obvious to me why those pages even need json.txt.

Stack trace probably isn't useful, but here's the end:

  File "/usr/lib/pymodules/python2.6/genshi/template/base.py", line 581, in _include
    cls=cls or self.__class__)
  File "/usr/lib/pymodules/python2.6/genshi/template/loader.py", line 227, in load
    filename, encoding=encoding)
  File "/usr/lib/pymodules/python2.6/genshi/template/loader.py", line 265, in _instantiate
    allow_exec=self.allow_exec)
  File "/usr/lib/pymodules/python2.6/genshi/template/base.py", line 379, in __init__
    raise TemplateSyntaxError(e.msg, self.filepath, e.lineno, e.offset)
TemplateSyntaxError: not well-formed (invalid token): line 1, column 0 (/usr/local/lib/python2.6/dist-packages/Bitten-0.7dev-py2.6.egg/bitten/templates/json.txt, line 1)

Running Bitten [857].

comment:27 follow-up: Changed 9 years ago by osimons

I've seen that bug too, but haven't really looked much at the internals of this change. I think perhaps it is related to the Lint reports - at least that is my hunch. I don't get the Lint graph on the config front page, and a build with Lint data won't render throwing the Exception in comment:26.

Walter, do you have Lint data in the builds you can't render?

David, that json.txt template do feel very 'hackish'... Why is is made like that?

comment:28 Changed 9 years ago by dfraser

My bad. When I was fixing the tests I blindly replaced something to use json.txt that was actually the test results renderer. Fix coming now...

json.txt is very hackish and was made like that because the chart summarizer API demands a template - we can look at changing that...

comment:29 follow-up: Changed 9 years ago by dfraser

wbell: Fix committed in [861] and backported in [862]

comment:30 in reply to: ↑ 27 ; follow-up: Changed 9 years ago by dfraser

Replying to osimons:

I've seen that bug too, but haven't really looked much at the internals of this change. I think perhaps it is related to the Lint reports - at least that is my hunch. I don't get the Lint graph on the config front page, and a build with Lint data won't render throwing the Exception in comment:26.

Can you look at the build/$buildname/chart/lint URL directly and see what that returns? I think this is a separate issue

comment:31 in reply to: ↑ 29 Changed 9 years ago by anonymous

Replying to dfraser:

wbell: Fix committed in [861] and backported in [862]

Looks good-- build pages are back.

comment:32 in reply to: ↑ 30 ; follow-up: Changed 9 years ago by osimons

Replying to dfraser:

Can you look at the build/$buildname/chart/lint URL directly and see what that returns? I think this is a separate issue

Sorry, false alarm. A result of a flaky development setup that seems to have rebuilt my "Lint" config using a recipe without working lint. Fixed that, rebuilt, and graphs display. Please ignore :-)

Also verified the latest fix as well, as all builds are displayed as expected.

Nice work! I'll now have a look at the json.txt template thingy.

comment:33 in reply to: ↑ 32 Changed 9 years ago by osimons

Replying to osimons:

I'll now have a look at the json.txt template thingy.

Reusing the Trac infrastructure, this seems to be the right way. It may look a bit odd, but the only other option is to write the request ourselves and raise RequestDone. Which we already do in bitten.master.BuildMaster._send_response() of course. We could refactor that into a shared utility function (along with the error-helper), and do away the need for a template and the rendering overhead.

Changed 9 years ago by hodgestar

Add config option for chart style attribute (allows chart width and height to be configured).

comment:34 follow-up: Changed 9 years ago by hodgestar

The new charts are a bit small for me but I realise some configuration pages are pressed for space so I add a configuration option for setting the chart style (and thus the width and height). I thought about adding something for just setting the width and height but setting the style seemed cleaner and more flexible (although I'm not completely sold on this).

Thoughts?

comment:35 Changed 9 years ago by anatoly techtonik <techtonik@…>

My thought is that configuration file is big enough even without styles, and any style modifications should belong to template customization level. The recipe can go into http://bitten.edgewall.org/wiki/AddingCharts and Adding Charts probably merged into documentation.

comment:36 Changed 9 years ago by anonymous

Overriding the report template wouldn't help here because the relevant style is part of the build config template. For the record, this would be the 9th Bitten configuration option and the user is free to ignore it.

Aside: The advice on the Adding Charts page is pretty horrendous. It would be better to create a tiny Trac plugin that registers its own entry points than to hack up Bitten for this purpose.

comment:37 follow-up: Changed 9 years ago by anatoly techtonik <techtonik@…>

BTW, where is the official documentation on enabling charts in 0.6b2? In my installation they doesn't render.

comment:38 in reply to: ↑ 37 Changed 9 years ago by dfraser

Replying to anatoly techtonik <techtonik@…>:

BTW, where is the official documentation on enabling charts in 0.6b2? In my installation they doesn't render.

There isn't any AFAIK. Are you referring to 0.6b2 or current svn? (They're vastly different) The charts should be automatically created if you have output that can be rendered by the appropriate chart creators. If they aren't (and I'm aware that there are issues with this sometimes), you can look at the generated chart data at http://$tracbaseurl/build/$buildname/chart/$chartcategory (where $chartcategory is test, lint, or coverage) - if that returns an error, or there is no data, then the chart will currently disappear

comment:39 Changed 9 years ago by anatoly techtonik <techtonik@…>

Thanks for clarification. Now I see that test results charts are not generated, because the test results were not provided. I need to see which XML format is more simple for implementation.

comment:40 in reply to: ↑ 34 Changed 9 years ago by hodgestar

I committed the resizable charts patch in r892 and backported it to 0.6.x in r893.

comment:41 Changed 9 years ago by hodgestar

  • Resolution set to fixed
  • Status changed from assigned to closed

I think we can declare this ticket closed -- any problems that arise now deserve their own tickets. Welcome to a completely open and free Bitten. :D

Add Comment

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain dfraser.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.