Skip to content

Fix DSS API documentation#227

Merged
kislyuk merged 1 commit intomasterfrom
tsmith-fix-docs2
Feb 7, 2019
Merged

Fix DSS API documentation#227
kislyuk merged 1 commit intomasterfrom
tsmith-fix-docs2

Conversation

@Bento007
Copy link
Copy Markdown
Collaborator

@Bento007 Bento007 commented Feb 5, 2019

Instantiate DSSClient to generate the documentation for swagger API binding

@Bento007 Bento007 self-assigned this Feb 5, 2019
@Bento007 Bento007 requested a review from kislyuk February 5, 2019 00:04
@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 5, 2019

Codecov Report

Merging #227 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #227      +/-   ##
=========================================
+ Coverage   83.39%   83.4%   +0.01%     
=========================================
  Files          37      37              
  Lines        1554    1555       +1     
=========================================
+ Hits         1296    1297       +1     
  Misses        258     258
Impacted Files Coverage Δ
hca/dss/__init__.py 84.04% <100%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ced76f4...971ac98. Read the comment docs.

Copy link
Copy Markdown
Member

@kislyuk kislyuk left a comment

Choose a reason for hiding this comment

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

I think the way to fix this is not by introducing new import-time side effects, but to make sure instantiation of the client happens in docs/conf.py. I think the regression happened in the following diff, and just reverting its effect should fix this:

https://github.com/HumanCellAtlas/dcp-cli/pull/146/files#diff-85987f48f1258d9ee486e3191495582dL25

You might consider inserting a comment to not accidentally remove these imports again.

@kislyuk
Copy link
Copy Markdown
Member

kislyuk commented Feb 5, 2019

(Also, in re: #146, whenever changing the doc build process, a good practice is to diff the output of the old build vs. the new.)

Instantiating DSS_Client in the doc/config.py ensures DSS swagger.yml APIs is parsed and included in the class before the sphinx parser runs.
@Bento007 Bento007 requested a review from kislyuk February 6, 2019 17:18
@kislyuk kislyuk merged commit 926de7f into master Feb 7, 2019
@kislyuk kislyuk deleted the tsmith-fix-docs2 branch February 7, 2019 20:19
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.

3 participants