ga4gh / ga4gh-server

Reference implementation of the APIs defined in ga4gh-schemas. RETIRED 2018-01-24
http://ga4gh.org
Apache License 2.0
96 stars 93 forks source link

Read the Docs (RTD) build failing #1599

Closed david4096 closed 7 years ago

david4096 commented 7 years ago

Facing same issue as in: https://github.com/ga4gh/schemas/issues/819

dcolligan commented 7 years ago

Looks like an RTD issue: https://github.com/rtfd/readthedocs.org/issues/2651

dcolligan commented 7 years ago

I suppose one option would be to get RTD's commercial support for this issue: https://readthedocs.com/services/#open-source-support Of course, both RTD yaml config and conda support are both beta features...

david4096 commented 7 years ago

Another option, hosting them off of a docs.ga4gh.org (or ucsc.edu) domain and deploying static HTML instead of recreating the sphinx environment.

dcolligan commented 7 years ago

Their code is open source so we could always fix the bug for them, heh. Although I suspect it's more of a configuration issue on their side than a problem with the code per se.

kozbo commented 7 years ago

@dcolligan did you have another approach using pip?

david4096 commented 7 years ago

@dcolligan yeah I just emailed https://github.com/ericholscher out of desperation. They probably need to update some local docker registry or some such thing.

@kozbo I believe we can make pip work for the release in all repos since we are now including compiled proto everywhere. But I don't think it will last if we accept the proposed change at https://github.com/ga4gh/schemas/issues/828 so we'll be thrashing back as soon as we want to build docs in RTD since we are going to require protoc to compile from source during development.

Another option: use a deploy script at travis to deploy a version of the schemas that includes the pb2 files on merged pull request. This would allow the downstream repos to use a "released" version that doesn't have the protoc requirement. If you tried to pip install git//ga4gh/schemas it would fail if you don't have protoc, but pip install ga4gh-schemas --pre will install a version with the proto installed.

Our internal dependencies should NOT have to build the proto from source. If we can do continuous release with a line or two of travis, I think that's the best option and solves https://github.com/ga4gh/schemas/issues/828 as well.

david4096 commented 7 years ago

This should now be fixed. New issue for discussing taking out RTD. Please close after rerunning.

dcolligan commented 7 years ago

Still not working, but it seems like a problem with our code now (importing bigwig), which is good.

https://readthedocs.org/projects/ga4gh-reference-implementation/builds/5120676/

dcolligan commented 7 years ago

Ok, made some progress (added bigwig to the pip requirements), but now stuck on this error:

https://readthedocs.org/projects/ga4gh-reference-implementation/builds/5120992/

(I pointed the RTD builds to my repo for the purposes of testing: https://github.com/dcolligan/server )

Even though curl is listed as a dependency, it doesn't seem to be installed before conda attempts to setup bigwig and it fails because it can't find curl-config.

@david4096 Do you have any suggestions for fixing this? Also, is there any guide to the environment.yml file format syntax?

david4096 commented 7 years ago

Create Environment File By Hand

That binary is provided by the libcurl-openssl-dev package in the server. I did a quick search of conda channels for it and didn't find anything.

I think we should be able to use PyBigWig without curl, but I'm not quite sure how. There might be a previous version that is compatible or another package out there.

https://github.com/dpryan79/pyBigWig/issues/29

dcolligan commented 7 years ago

Thanks for filing that issue with pyBigWig.

Another route would be creating a conda package ourselves that included curl-config... I'm not sure how feasible this is, knowing little about conda.

Yet another thing to do would be to comment out portions of the code that include bigwig if we detect that we are on RTD: http://docs.readthedocs.io/en/latest/faq.html#how-do-i-change-behavior-for-read-the-docs

That seems the most feasible -- although cumbersome and hacky -- option at this point, since ditching RTD, the conda build, or BigWig all seem unacceptable... thoughts?

david4096 commented 7 years ago

If it's a sphinx feature it's not weird or hacky as far as they're concernred.

The next section let's you mock C-modules. Can we do it that way?

There's no reason to actually have pybigwig around for doc generation since we just generate docs for CLI portions, that don't depend on it. We don't really need to have pybigwig around (or a lot of other things). If the conditional approach is pretty easy, we can use that.

dpryan79 commented 7 years ago

I've posted instructions on how to mock dependencies on the issue @david4096 posted on pyBigWig. I'll add that you could just install it through conda too, since that's what you're using.

david4096 commented 7 years ago

Thanks @dpryan79 https://github.com/dpryan79/pyBigWig/issues/29#issuecomment-285497043

dcolligan commented 7 years ago

Fixed in #1614