Skip to content

Conversation

@Dieterbe
Copy link
Contributor

@Dieterbe Dieterbe commented Sep 2, 2014

similar to previous PR, except now it integrates into the
main yaml file instead of a separate graphTemplates.conf

@brutasse
Copy link
Owner

brutasse commented Sep 2, 2014

Nice! It looks like a minor edit is needed in docs/api.html as well. It still mentions graphTemplates.conf.

Do you think you can add a test? Maybe with SVG rendering to make sure colors are set properly.

For greater graphite-web compatibility we should probably provide the same templates as here by default. What do you think?

@Dieterbe
Copy link
Contributor Author

Dieterbe commented Sep 2, 2014

i personally think all the template stuff in the main yaml is obtrusive. the less template stuff in the main yaml, the better as far as i'm concerned.

maybe all that stuff could go in a separate yaml file, but let's see if anyone actually expresses an interest in having those default templates available.

@brutasse
Copy link
Owner

brutasse commented Sep 2, 2014

I meant having a templates section in default_conf in graphite_api/config.py. For people needing graphite-web's vanilla templates, they wouldn't have any configuration to do because templates would already be there (you can run graphite-api without a config file if your setup works with the defaults). And for people needing custom templates, they can simply add the section in the yaml.

But I care more about testing than providing an extensive suite of templates :)

@Dieterbe
Copy link
Contributor Author

Dieterbe commented Sep 2, 2014

Ah, ok yeah that makes sense.
I've been looking at the testing code. This is going to take me a while to figure out what the best way is to test this properly, and i don't really have much time remaining :(

Dieter Plaetinck added 2 commits December 11, 2014 17:37
similar to previous PR, except now it integrates into the
main yaml file instead of a separate graphTemplates.conf
@Dieterbe
Copy link
Contributor Author

rebased and pushed a new fix.

@brutasse so the test should be something like the following?

    def test_render_template(self):
        whisper.create(self.db, [(1, 60)])
        from pprint import pprint
        self.app.config = {'templates': {'foo': {'linecolors': ['#aaaaaa', '#bbbbbb']}}}
        response = self.app.get(self.url, query_string={'target': 'test',
                                                        'format': 'svg'})
        # assert here that somewhere in lines we should see the first color being used for the target?
        lines = response.data.decode('utf-8').strip().split('\n')
        pprint(lines)

unfortunately the svg output doesn't contain the color anywhere :( not sure what's wrong, as the template colors work fine with png output. the lines pprint output is here: https://gist.github.com/Dieterbe/affcdc35e24c17c219f5

@Dieterbe
Copy link
Contributor Author

@brutasse bump. would love to get this merged. i attempted a unit test but i'm still stuck (see above). thanks.

@axos88
Copy link

axos88 commented Feb 2, 2016

Please merge this @brutasse

@zzl0
Copy link

zzl0 commented Nov 16, 2016

👍

@zzl0
Copy link

zzl0 commented Nov 22, 2016

@brutasse @Dieterbe What's the status of this PR?

@zzl0 zzl0 mentioned this pull request Nov 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants