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

[WIP] Generate dashboard code from json #91

Closed
wants to merge 40 commits into from

Conversation

atopuzov
Copy link
Contributor

@atopuzov atopuzov commented Nov 1, 2017

This is still WIP. Use for discussion on direction.

Tries to solve: #75
Tested with https://github.com/percona/grafana-dashboards
Had some success at "beautifying" the output with https://pythoniter.appspot.com/

@atopuzov atopuzov force-pushed the from_json branch 3 times, most recently from 50623ea to 2bef351 Compare November 3, 2017 11:23
@jml jml changed the title Generate dashboard code from json [WIP] Generate dashboard code from json Nov 10, 2017
@jml jml added the wip label Nov 13, 2017
@jml
Copy link
Contributor

jml commented Nov 13, 2017

Thanks! I'm excited about this. I think the approach of parsing JSON into Grafanalib objects and then pretty-printing those is the right one.

I'm going to insist that we have Hypothesis-powered roundtripping tests for this code. That's going to be a lot of work, I'm afraid. I'm happy to chip in, but I'm unlikely to go as fast as you'd like.

@atopuzov
Copy link
Contributor Author

@jml Can you provide a small example of hypotestis testing and I can take from there (I haven't used it before).

@jml
Copy link
Contributor

jml commented Nov 14, 2017

from hypothesis import given
from hypothesis import strategies as st

def colors():
    """Generate arbitrary value from 0 to 255, for RGB color."""
    return st.integers(min_value=0, max_value=255)

def rgbas():
    """Generate arbitrary, valid RGBA objects."""
    return st.build(RGBA, r=colors(), g=colors(), b=colors(), a=st.floats(min_value=0, max_value=1)

@given(x=rbgas())
def test_rgbas_roundtrips(x):
    """RGBA can be encoded and then decoded."""
    json_dict = x.to_json_data()
    y = RGBA.parse_json_data(json_dict)
    assert x == y

I think if you use classmethod rather than staticmethod you can do x.parse_json_data, which would allow you to extract a re-usable roundtripping tester, but my knowledge of Python minutiae is a little stale.

Does that help?

@atopuzov
Copy link
Contributor Author

@jml Awesome! Thanks. Will take it for here as time allows.

@atopuzov
Copy link
Contributor Author

@jml Managed to do some hypotesis testing (which is awesome, just like haskell quickcheck).
Needs a bit of cleanup and making the things generated a bit stricter. How does it look to you?

@jml
Copy link
Contributor

jml commented Nov 21, 2017

Oh wow, this is great, thank you.

Some thoughts:

  • I'm kind of fussy about Python formatting. If you could make this match the rest of grafanalib, lovely. If not, I'll happily do the re-indenting myself.
  • I'd rather we not use destructive operations like pop on things that get passed to us, reserving it for values that only live inside a single scope.
  • Rather than using @staticmethod and then manually stating the class name (e.g. return Template(*args, **kwargs)), why not use @classmethod and then return cls(*args, **kwargs)?

I'll follow up with some specific in-line comments.

(Also, yes, Hypothesis is awesome. I love Haskell and I love QuickCheck, but Hypothesis is way better, IMHO.)

Copy link
Contributor

@jml jml left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So grateful that you're tackling this.

@@ -66,7 +66,7 @@ lint: $(VIRTUALENV_BIN)/flake8
$(VIRTUALENV_BIN)/flake8 gfdatasource/gfdatasource grafanalib

test: $(VIRTUALENV_BIN)/py.test
$(VIRTUALENV_BIN)/py.test --junitxml=$(JUNIT_XML)
$(VIRTUALENV_BIN)/py.test --junitxml=$(JUNIT_XML) --cov=grafanalib --cov-report term-missing

This comment was marked as abuse.

This comment was marked as abuse.

# parser.add_argument(
# '--output', '-o', type=os.path.abspath,
# help='Where to write the dashboard JSON'
# )

This comment was marked as abuse.

This comment was marked as abuse.


# def targets():
# return st.builds(G.Target,
# # target param is usde by st.builds ;-(

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.


def xaxes():
return st.builds(G.XAxis,
# XAxis loses information

This comment was marked as abuse.

This comment was marked as abuse.

json_data = io.StringIO()
_gen.write_dashboard(obj, json_data)
json_data.seek(0)
return json.load(json_data)

This comment was marked as abuse.

This comment was marked as abuse.

if object_type == DATASOURCE_TYPE:
return DataSourceInput.parse_json_data(data)
elif object_type == CONSTANT_TYPE:
return ConstantInput.parse_json_data(data)

This comment was marked as abuse.

This comment was marked as abuse.

if 'datasource' in data:
data['dataSource'] = data.pop('datasource')

# Deleted values

This comment was marked as abuse.

This comment was marked as abuse.

elif panel_type == SINGLESTAT_TYPE:
return SingleStat.parse_json_data(panel)
elif panel_type == TABLE_TYPE:
return Table.parse_json_data(panel)

This comment was marked as abuse.

This comment was marked as abuse.

if 'links' in data:
data['links'] = [DashboardLink.parse_json_data(link)
for link in data['links']]

This comment was marked as abuse.

This comment was marked as abuse.



@attr.s
class Table(object):

This comment was marked as abuse.

ncabatoff pushed a commit to ncabatoff/grafanalib that referenced this pull request Nov 22, 2017
35679ee Merge pull request weaveworks#110 from weaveworks/parallel-push-errors
3ae41b6 Remove unneeded if block
51ff31a Exit on first error
0faad9f Check for errors when pushing images in parallel
74dc626 Merge pull request weaveworks#108 from weaveworks/disable-apt-daily
b4f1d91 Merge pull request weaveworks#107 from weaveworks/docker-17-update
7436aa1 Override apt daily job to not run immediately on boot
7980f15 Merge pull request weaveworks#106 from weaveworks/document-docker-install-role
f741e53 Bump to Docker 17.06 from CE repo
61796a1 Update Docker CE Debian repo details
0d86f5e Allow for Docker package to be named docker-ce
065c68d Document selection of Docker installation role.
3809053 Just --porcelain; it defaults to v1
11400ea Merge pull request weaveworks#105 from weaveworks/remove-weaveplugin-remnants
b8b4d64 remove weaveplugin remnants
35099c9 Merge pull request weaveworks#104 from weaveworks/pull-docker-py
cdd48fc Pull docker-py to speed tests/builds up.
e1c6c24 Merge pull request weaveworks#103 from weaveworks/test-build-tags
d5d71e0 Add -tags option so callers can pass in build tags
8949b2b Merge pull request weaveworks#98 from weaveworks/git-status-tag
ac30687 Merge pull request weaveworks#100 from weaveworks/python_linting
4b125b5 Pin yapf & flake8 versions
7efb485 Lint python linting function
444755b Swap diff direction to reflect changes required
c5b2434 Install flake8 & yapf
5600eac Lint python in build-tools repo
0b02ca9 Add python linting
c011c0d Merge pull request weaveworks#79 from kinvolk/schu/python-shebang
6577d07 Merge pull request weaveworks#99 from weaveworks/shfmt-version
00ce0dc Use git status instead of diff to add 'WIP' tag
411fd13 Use shfmt v1.3.0 instead of latest from master.
0d6d4da Run shfmt 1.3 on the code.
5cdba32 Add sudo
c322ca8 circle.yml: Install shfmt binary.
e59c225 Install shfmt 1.3 binary.
30706e6 Install pyhcl in the build container.
960d222 Merge pull request weaveworks#97 from kinvolk/alban/update-shfmt-3
1d535c7 shellcheck: fix escaping issue
5542498 Merge pull request weaveworks#96 from kinvolk/alban/update-shfmt-2
32f7cc5 shfmt: fix coding style
09f72af lint: print the diff in case of error
571c7d7 Merge pull request weaveworks#95 from kinvolk/alban/update-shfmt
bead6ed Update for latest shfmt
b08dc4d Update for latest shfmt (weaveworks#94)
2ed8aaa Add no-race argument to test script (weaveworks#92)
80dd78e Merge pull request weaveworks#91 from weaveworks/upgrade-go-1.8.1
08dcd0d Please ./lint as shfmt changed its rules between 1.0.0 and 1.3.0.
a8bc9ab Upgrade default Go version to 1.8.1.
41c5622 Merge pull request weaveworks#90 from weaveworks/build-golang-service-conf
e8ebdd5 broaden imagetag regex to fix haskell build image
ba3fbfa Merge pull request weaveworks#89 from weaveworks/build-golang-service-conf
e506f1b Fix up test script for updated shfmt
9216db8 Add stuff for service-conf build to build-goland image
66a9a93 Merge pull request weaveworks#88 from weaveworks/haskell-image
cb3e3a2 shfmt
74a5239 Haskell build image
4ccd42b Trying circle quay login
b2c295f Merge branch 'common-build'
0ac746f Trim quay prefix in circle script
c405b31 Merge pull request weaveworks#87 from weaveworks/common-build
9672d7c Push build images to quay as they have sane robot accounts
a2bf112 Review feedback
fef9b7d Add protobuf tools
10a77ea Update readme
254f266 Don't need the image name in
ffb59fc Adding a weaveworks/build-golang image with tags
b817368 Update min Weave Net docker version
cf87ca3 Merge pull request weaveworks#86 from weaveworks/lock-kubeadm-version
3ae6919 Add example of custom SSH private key to tf_ssh's usage.
cf8bd8a Add example of custom SSH private key to tf_ansi's usage.
c7d3370 Lock kubeadm's Kubernetes version.
faaaa6f Merge pull request weaveworks#84 from weaveworks/centos-rhel
ef552e7 Select weave-kube YAML URL based on K8S version.
b4c1198 Upgrade default kubernetes_version to 1.6.1.
b82805e Use a fixed version of kubeadm.
f33888b Factorise and make kubeconfig option optional.
f7b8b89 Install EPEL repo for CentOS.
615917a Fix error in decrypting AWS access key and secret.
86f97b4 Add CentOS 7 AMI and username for AWS via Terraform.
eafd810 Add tf_ansi example with Ansible variables.
2b05787 Skip setup of Docker over TCP for CentOS/RHEL.
84c420b Add docker-ce role for CentOS/RHEL.
00a820c Add setup_weave-net_debug.yml playbook for user issues' debugging.
3eae480 Upgrade default kubernetes_version to 1.5.4.
753921c Allow injection of Docker installation role.
e1ff90d Fix kubectl taint command for 1.5.
b989e97 Fix typo in kubectl taint for single node K8S cluster.
541f58d Remove 'install_recommends: no' for ethtool.
c3f9711 Make Ansible role docker-from-get.docker.com work on RHEL/CentOS.
038c0ae Add frequently used OS images, for convenience.
d30649f Add --insecure-registry to docker.conf
1dd9218 shfmt -i 4 -w push-images
6de96ac Add option to not push docker hub images
310f53d Add push-images script from cortex
8641381 Add port 6443 to kubeadm join commands for K8S 1.6+.
50bf0bc Force type of K8S token to string.
08ab1c0 Remove trailing whitespaces.
ae9efb8 Enable testing against K8S release candidates.
9e32194 Secure GCP servers for Scope: open port 80.
a22536a Secure GCP servers for Scope.
89c3a29 Merge pull request weaveworks#78 from weaveworks/lint-merge-rebase-issue-in-docs
73ad56d Add linter function to avoid bad merge/rebase artefact
31d069d Change Python shebang to `#!/usr/bin/env python`
52d695c Merge pull request weaveworks#77 from kinvolk/schu/fix-relative-weave-path
77aed01 Merge pull request weaveworks#73 from weaveworks/mike/sched/fix-unicode-issue
7c080f4 integration/sanity_check: disable SC1090
d6d360a integration/gce.sh: update gcloud command
e8def2c provisioning/setup: fix shellcheck SC2140
cc02224 integration/config: fix weave path
9c0d6a5 Fix config_management/README.md
334708c Merge pull request weaveworks#75 from kinvolk/alban/external-build-1
da2505d gce.sh: template: print creation date
e676854 integration tests: fix user account
8530836 host nameing: add repo name
b556c0a gce.sh: fix deletion of gce instances
2ecd1c2 integration: fix GCE --zones/--zone parameter
3e863df sched: Fix unicode encoding issues
51785b5 Use rm -f and set current dir using BASH_SOURCE.
f5c6d68 Merge pull request weaveworks#71 from kinvolk/schu/fix-linter-warnings
0269628 Document requirement for `lint_sh`
9a3f09e Fix linter warnings
efcf9d2 Merge pull request weaveworks#53 from weaveworks/2647-testing-mvp
d31ea57 Weave Kube playbook now works with multiple nodes.
27868dd Add GCP firewall rule for FastDP crypto.
edc8bb3 Differentiated name of dev and test playbooks, to avoid confusion.
efa3df7 Moved utility Ansible Yaml to library directory.
fcd2769 Add shorthands to run Ansible playbooks against Terraform-provisioned virtual machines.
f7946fb Add shorthands to SSH into Terraform-provisioned virtual machines.
aad5c6f Mention Terraform and Ansible in README.md.
dddabf0 Add Terraform output required for templates' creation.
dcc7d02 Add Ansible configuration playbooks for development environments.
f86481c Add Ansible configuration playbooks for Docker, K8S and Weave-Net.
efedd25 Git-ignore Ansible retry files.
765c4ca Add helper functions to setup Terraform programmatically.
801dd1d Add Terraform cloud provisioning scripts.
b8017e1 Install hclfmt on CircleCI.
4815e19 Git-ignore Terraform state files.
0aaebc7 Add script to generate cartesian product of dependencies of cross-version testing.
007d90a Add script to list OS images from GCP, AWS and DO.
ca65cc0 Add script to list relevant versions of Go, Docker and Kubernetes.
aa66f44 Scripts now source dependencies using absolute path (previously breaking make depending on current directory).
7865e86 Add -p option to parallelise lint.
36c1835 Merge pull request weaveworks#69 from weaveworks/mflag
9857568 Use mflag and mflagext package from weaveworks/common.
9799112 Quote bash variable.
10a36b3 Merge pull request weaveworks#67 from weaveworks/shfmt-ignore
a59884f Add support for .lintignore.
03cc598 Don't lint generated protobuf code.
2b55c2d Merge pull request weaveworks#66 from weaveworks/reduce-test-timeout
d4e163c Make timeout a flag
49a8609 Reduce test timeout
8fa15cb Merge pull request weaveworks#63 from weaveworks/test-defaults
b783528 Tweak test script so it can be run on a mca
a3b18bf Merge pull request weaveworks#65 from weaveworks/fix-integration-tests
ecb5602 Fix integration tests
f9dcbf6 ... without tab (clearly not my day)
a6215c3 Add break I forgot
0e6832d Remove incorrectly added tab
eb26c68 Merge pull request weaveworks#64 from weaveworks/remove-test-package-linting
f088e83 Review feedback
2c6e83e Remove test package linting
2b3a1bb Merge pull request weaveworks#62 from weaveworks/revert-61-test-defaults
8c3883a Revert "Make no-go-get the default, and don't assume -tags netgo"
e75c226 Fix bug in GC of firewall rules.
e49754e Merge pull request weaveworks#51 from weaveworks/gc-firewall-rules
191f487 Add flag to enale/disable firewall rules' GC.
567905c Add GC of firewall rules for weave-net-tests to scheduler.
03119e1 Fix typo in GC of firewall rules.
bbe3844 Fix regular expression for firewall rules.
c5c23ce Pre-change refactoring: splitted gc_project function into smaller methods for better readability.
ed5529f GC firewall rules
ed8e757 Merge pull request weaveworks#61 from weaveworks/test-defaults
57856e6 Merge pull request weaveworks#56 from weaveworks/remove-wcloud
dd5f3e6 Add -p flag to test, run test in parallel
62f6f94 Make no-go-get the default, and don't assume -tags netgo
8946588 Merge pull request weaveworks#60 from weaveworks/2647-gc-weave-net-tests
4085df9 Scheduler now also garbage-collects VMs from weave-net-tests.
4b7d5c6 Merge pull request weaveworks#59 from weaveworks/57-fix-lint-properly
b7f0e69 Merge pull request weaveworks#58 from weaveworks/fix-lint
794702c Pin version of shfmt
ab1b11d Fix lint
d1a5e46 Remove wcloud cli tool

git-subtree-dir: tools
git-subtree-split: 35679ee5ff17c4edf864b7c43dc70a40337fcd80
@dogopupper
Copy link

Can this be tested / instructions for doing so? @atopuzov

@atopuzov
Copy link
Contributor Author

atopuzov commented Dec 1, 2017

@dogopupper you can checkout the branch from my repo and pip install it.
eg.

git clone [email protected]:atopuzov/grafanalib.git
git checkout from_json
pip install . # I suggest you do it in a virtualenv
parse-dashboard <json-file> -o <python-file>

Obviously it is dependent on what is implemented in grafanalib atm.

@atopuzov
Copy link
Contributor Author

atopuzov commented Dec 1, 2017

@jml I think I have addressed all the concerns you had. Can you give it another look? Thanks!

@jml
Copy link
Contributor

jml commented Dec 4, 2017 via email

@dogopupper
Copy link

dogopupper commented Dec 4, 2017

@atopuzov @jml having trouble with importlib.machinery SourceFileLoader when running parse-dashboard

https://docs.python.org/3/library/importlib.html#importlib.machinery.SourceFileLoader

  File "/usr/local/bin/parse-dashboard", line 9, in <module>
    load_entry_point('grafanalib==0.3.0', 'console_scripts', 'parse-dashboard')()
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/pkg_resources/__init__.py", line 565, in load_entry_point
    return get_distribution(dist).load_entry_point(group, name)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/pkg_resources/__init__.py", line 2697, in load_entry_point
    return ep.load()
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/pkg_resources/__init__.py", line 2370, in load
    return self.resolve()
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/pkg_resources/__init__.py", line 2376, in resolve
    module = __import__(self.module_name, fromlist=['__name__'], level=0)
  File "/Library/Python/2.7/site-packages/grafanalib/_gen.py", line 9, in <module>
    from importlib.machinery import SourceFileLoader
ImportError: No module named machinery```

@dogopupper
Copy link

when using withn python 3.6:

  File "/Users/dogopupper/work/grafanalib/p35/bin/parse-dashboard", line 11, in <module>
    load_entry_point('grafanalib==0.3.0', 'console_scripts', 'parse-dashboard')()
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/_gen.py", line 159, in parse_dashboard_script
    run_script(parse_dashboard)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/_gen.py", line 120, in run_script
    sys.exit(f(sys.argv[1:]))
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/_gen.py", line 151, in parse_dashboard
    dashboard = grafanalib.core.Dashboard.parse_json_data(json_data)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 1213, in parse_json_data
    transform=foreach(DashboardLink.parse_json_data)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 54, in transform_dict
    new_data[new_key] = transform(old_value)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 60, in inner
    return [func(arg) for arg in args]
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 60, in <listcomp>
    return [func(arg) for arg in args]
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 642, in parse_json_data
    dicttransform('height', transform=Pixels.parse_json_data)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 54, in transform_dict
    new_data[new_key] = transform(old_value)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 60, in inner
    return [func(arg) for arg in args]
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 60, in <listcomp>
    return [func(arg) for arg in args]
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 640, in <lambda>
    transform=foreach(lambda pnl: parse_object(pnl, PANEL_TYPES))
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 1756, in parse_object
    return mapping[object_type].parse_json_data(obj)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 1663, in parse_json_data
    transform=foreach(ValueMap.parse_json_data)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 54, in transform_dict
    new_data[new_key] = transform(old_value)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 60, in inner
    return [func(arg) for arg in args]
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 60, in <listcomp>
    return [func(arg) for arg in args]
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 1420, in parse_json_data
    return cls(**data)
TypeError: __init__() got an unexpected keyword argument 'from'```

@atopuzov
Copy link
Contributor Author

atopuzov commented Dec 4, 2017

@dogopupper Found the bug, will push a fix. Thanks.

@dogopupper
Copy link

Thank you @atopuzov . New error (3.6):

  File "/Users/dogopupper/work/grafanalib/p35/bin/parse-dashboard", line 11, in <module>
    load_entry_point('grafanalib==0.3.0', 'console_scripts', 'parse-dashboard')()
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/_gen.py", line 159, in parse_dashboard_script
    run_script(parse_dashboard)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/_gen.py", line 120, in run_script
    sys.exit(f(sys.argv[1:]))
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/_gen.py", line 151, in parse_dashboard
    dashboard = grafanalib.core.Dashboard.parse_json_data(json_data)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 1213, in parse_json_data
    transform=foreach(DashboardLink.parse_json_data)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 54, in transform_dict
    new_data[new_key] = transform(old_value)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 60, in inner
    return [func(arg) for arg in args]
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 60, in <listcomp>
    return [func(arg) for arg in args]
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 642, in parse_json_data
    dicttransform('height', transform=Pixels.parse_json_data)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 54, in transform_dict
    new_data[new_key] = transform(old_value)
  File "/Users/dogopupper/work/grafanalib/p35/lib/python3.6/site-packages/grafanalib/core.py", line 133, in parse_json_data
    match = Pixels.REGEX.match(data)
TypeError: expected string or bytes-like object```

Would it be best if I passed my 3k lines JSON with multiple graphs to you so you can further develop the solution until it works?

@atopuzov
Copy link
Contributor Author

atopuzov commented Dec 4, 2017

@dogopupper If you can drop it somewhere would be great. Eg. a github gist, pastebin.

@dogopupper
Copy link

https://pastebin.com/QGbYPL6m 🍰

@atopuzov
Copy link
Contributor Author

atopuzov commented Dec 5, 2017

@dogopupper I tested with your input and now the only issue is when the row height is null or empty string.
Looking at 1 for Row it has to be an instance of Pixels. There is also DEFAULT_ROW_HEIGHT which is 250. height is also defined as an attribute on Graph, SingleStat, Text and Table but it can be None there.I made it so that for Row it can be None as well through att.validators.optional (i should also make it that other object do the same).
At the moment you need to remove __requires from the JSON as grafanalib does not have necessary objects to represent that. Unfortunately your dashboard is not accessible anymore and I edited the one I downloaded.

[1] https://github.com/weaveworks/grafanalib/blob/master/grafanalib/core.py#L400

@dogopupper
Copy link

@atopuzov oooooo it works!!!!!!!!!!!! however..... the python result is 20% longer (800 extra lines, around 4100)...... it's like a huge yaml file with the indentation mess (after using the pythoniter beautify)...... is the resulting grafanalib code what is actually needed to produce the dashboard or is there a lot of redundant information that can be further worked on?

@atopuzov
Copy link
Contributor Author

atopuzov commented Dec 5, 2017

@dogopupper Yes there is a lot of redundant information (eg. defaults). The object is just pretty printed to a file (eg. you would get the same result for any dashboard you define with the minimum of python code and pretty print it to file, it would also be huge).
For the moment I haven't actually thought of a way to deal with that.

@@ -596,7 +596,8 @@ class Row(object):
editable = attr.ib(
default=True, validator=instance_of(bool),
)
height = attr.ib(default=DEFAULT_ROW_HEIGHT, validator=instance_of(Pixels))
height = attr.ib(default=DEFAULT_ROW_HEIGHT,
validator=optional(instance_of(Pixels)))

This comment was marked as abuse.

This comment was marked as abuse.

@jml
Copy link
Contributor

jml commented Dec 5, 2017

Yes there is a lot of redundant information (eg. defaults). The object is just pretty printed to a file (eg. you would get the same result for any dashboard you define with the minimum of python code and pretty print it to file, it would also be huge). For the moment I haven't actually thought of a way to deal with that.

FWIW, I'm OK with this still being the case when we merge, and am also OK with doing a release in this state, as long as the command-line tool is marked as experimental.

Once we've done the hard work of turning the JSON into Python objects (thanks again @atopuzov!), manipulating those objects is relatively easy.

@atopuzov
Copy link
Contributor Author

@jml I need to rebase first and then it's ready for a final review.

@atopuzov atopuzov force-pushed the from_json branch 3 times, most recently from 2c4e55b to 83d742f Compare January 13, 2018 10:33
@atopuzov
Copy link
Contributor Author

@jml This should be good to go.

@jml
Copy link
Contributor

jml commented Jan 19, 2018

Thanks @atopuzov. I'll take a look as soon as I can. Might not be for a couple of weeks though.

@jml
Copy link
Contributor

jml commented Feb 27, 2018

Hi @atopuzov

I've got a local rebase of this on my laptop. Is there some way I can share this with you so you don't have to do the laborious work of rebasing yourself?

I had a brief look over the changes and I think I'd like to have a play around with the code you've written, rather than doing a traditional review.

Here's what I'll be looking into:

  • can commit history be cleaned up a bit?
  • can we add the Hypothesis stuff (or a simplified version of Hypothesis stuff) first in one PR and then the parser in another?

As well as general exploration to make sure I understand what's going on.

@anandsinghkunwar
Copy link

Any updates on this?

@atopuzov
Copy link
Contributor Author

Unfortunately I don't have the time ATM. I'll pick it up when time allows.
Sorry :-(

@jakewarr8
Copy link

What's the status on this?

@atopuzov atopuzov closed this Aug 28, 2018
@atopuzov
Copy link
Contributor Author

Sorry I can't commit any time to this.

@jml
Copy link
Contributor

jml commented Aug 29, 2018

No worries. Thanks for letting us know, and thank you for your efforts!

@psmgeelen
Copy link

Hi there, I would still love to have this feature, can I be supportive to this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants