http API

[nevrome]

Started to play with a server-rendered data explorer webpage.

[codecov]

Codecov Report

Attention: Patch coverage is 8.07799% with 330 lines in your changes missing coverage. Please review.

Project coverage is 55.67%. Comparing base (64b586d) to head (d45c543).
Report is 73 commits behind head on master.

Files with missing lines	Patch %	Lines
src/Poseidon/ServerHTML.hs	0.00%	185 Missing ⚠️
src/Poseidon/CLI/Serve.hs	14.83%	122 Missing and 10 partials ⚠️
src/Poseidon/CLI/OptparseApplicativeParsers.hs	0.00%	12 Missing ⚠️
src/Poseidon/ServerStylesheet.hs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #327      +/-   ##
==========================================
- Coverage   59.69%   55.67%   -4.02%     
==========================================
  Files          29       31       +2     
  Lines        4364     4688     +324     
  Branches      498      505       +7     
==========================================
+ Hits         2605     2610       +5     
- Misses       1261     1573     +312     
- Partials      498      505       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[stschiff]

Great, I'll review this ASAP.

[stschiff]

OK, this is really brillant. I've tested it on my local copy of the community-archive. I could not test the multiple-version feature, as the local copy of course has only the latest versions.

I left some comments about a more efficient implementation of getArchiveByName which would make prepPacs redundant. And I noticed a redundant test file. I mark this as "Comments", but I'm happy to approve it once you've checked those.

I know you've warned me about the hard-coded archive names in ServerHTML.hs. Now that I see them I must admit that I'm getting a slight cringe-feeling about having them in. I get it, and for pragmatic reasons I think maybe this is OK for now.

I have a suggestion to fix this, but I admit that it might be too much work for now, so maybe we can put that for the future:
The serve command could take an optional flag archivesFile, which would replace the current -d arch_name=path syntax by a TSV file. In that file, we could accept two basic columns, which is archive name and path, and then additional optional columns indicating a brief description for the website and github links and stuff. So this file could like this:

community-archive    /path/to/pca   "Poseidon Community Archive" "Poseidon Community Archive (PCA) with per-paper packages and genotype data as published." "https://github.com/poseidon-framework/community-archive"
aadr-archive     /path/to/paa     "Poseidon AADR Archive"     "Poseidon AADR Archive (PAA) with a structurally unified version of the AADR dataset repackaged in the Poseidon package format." "https://github.com/poseidon-framework/aadr-archive"

and maybe even additional columns to track whatever the server outputs specifically for the hard-coded names.

Again, I think I can't do this right now, as there are too many more important things, but this is a direction that I would prefer over the hard-coding. If you decide you also can't do it, then I suggest we leave it as is and move this comment into another github issue and tag it with "for the future" or so.

[nevrome]

Thanks for the review! The idea to extend/replace -d arch_name=path with a config file is neat. .csv is a bit of an odd choice here, and I would rather suggest .yml, but the direction is clear. I don't think this is too hard to implement, so maybe we should do it right away.

Please note that I'm on holiday next week and will only come back to this afterwards. But feel free to make the requested chances yourself, if you feel like it. I think we should then host this version on the test server first, to involve @TCLamnidis, @93Boy, and @Kavlahkaff in the testing process.

[nevrome]

OK - the test server is now running the first draft of the data explorer:

http://server.poseidon-adna.org:3000/explorer

I would like to merge this asap, so please tell me only about bugs in this PR. Any feature requests should go here: #333

Note that this test-sever version hides download links, because the test server traditionally doesn't include .zip archives.

[stschiff]

Very nice. I tried it out and I'm quite pleased. I had a rough look again through the code, but in the end I already reviewed all of this last time and in the other PR that was merged in. So from my side this is good to go!

[nevrome]

OK - thanks! Will merge and instate this version as the main server one.