search

divinity666 / ruby-grafana-reporter

Reporting Service for Grafana
MIT License
66 stars 5 forks source link

Implementing compression for erb generated reports #52

Open andrejshapal opened 3 weeks ago

andrejshapal commented 3 weeks ago

https://github.com/divinity666/ruby-grafana-reporter/issues/51

divinity666 commented 3 weeks ago

Thanks for your PR. I will check in more detail and come back then.

Regarding your questions, please open an issue, where we can then take the discussion

divinity666 commented 3 weeks ago

I just looked in more detail in this PR now. I think, we could by default create a ZIP file out of a ERB template. Anyway, this will have some drawbacks:

The ERB report templates can be used to achieve really EVERYTHING, that is possible with regular Ruby as well. In such a setup, it is not even clear, if e.g. image files are stored at an arbitrary place, or if anything else has been modified at any place in the file system. So it will NOT be possible to include everything else, that has been done with the ERB template, but only ZIP the result.

If that is, what you intend to do, I check on how to merge it.

UPDATE: I just read the issue in more detail. I now understand, that there is definitely an issue. But it would still be interesting, if you would like to get a ZIP out of the ERB template rendering, or rather the plain result.

andrejshapal commented 3 weeks ago

@divinity666 I think we have to move zip function out of specific report-class and re-use in both classes as you proposed above. Img inclusion in zip should be by default. Plain text could be a feature, but I don't see much sense in it at this point.

andrejshapal commented 3 weeks ago

@divinity666 I have refactored zip function. The thing which looks tricky in tests -o. The current implementation actually makes it to rewrite the zip file extension (or even pdf). I would use -o as path, --report_file_name and --report_file_extension File extension would be used only for erb for the file in zip. File name would be used fo any report. Path would be used as destination where to save report.

By the way, the test in pipeline fails while it works fine in my env. Probably something related to ci env.

divinity666 commented 3 weeks ago

Hm, I don't really like the idea to specify file extensions explicitly. I guess, we should find a way to avoid it.

Furthermore, the CI is failing, because your proposal does not work in the windows build, because of the different slashes used for folders. You should rather use a OS independent ruby constant. There should be one, as far as I recall.

Did you ensure, that PDF is not ZIPed?