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

Nightly build for documentation deployment #615

Merged
merged 9 commits into from
May 13, 2020
Merged

Conversation

dhruvbatra
Copy link
Contributor

@dhruvbatra dhruvbatra commented May 2, 2020

Motivation and Context

Our CI runs on every commit, but that means if a commit doesn't land in habitat-sim, our documentations (e.g. changes to habitat-api docs) don't update on aihabitat.org.

For instance, facebookresearch/habitat-lab#351 fixed an issue with our documentation and while it has landed into H-API master, aihabitat.org is still out of date.

This PR follows the nightly build example here:
https://circleci.com/docs/2.0/workflows/#nightly-example

to run the update_docs jobs in our CI.

This PR also (I believe, not sure) addresses the issue discussed here:
https://github.com/facebookmicrosites/habitat-website/pull/64

How Has This Been Tested

How does one test the testing script? Who watches the watchmen? 🤷
(Edit: Apparently CI will complain if you break dependency in the CI config, so we have an answer)

Types of changes

-- Added nightly workflow to update_docs job, following example here: https://circleci.com/docs/2.0/workflows/#nightly-example
-- Looks like a valid schedule requires a cron key and a filters key. So the master branch instructions are repeated.

-- Added nightly workflow to `update_docs` job, following example here: https://circleci.com/docs/2.0/workflows/#nightly-example
-- Looks like a valid schedule requires a cron key and a filters key. So the master branch instructions are repeated.
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label May 2, 2020
-- Fixing CI error.
@erikwijmans
Copy link
Contributor

I think the issue here isn't that the doc builder isn't running on habitat-API commits, it's that it doesn't build the docs for habitat-API at all: https://github.com/facebookresearch/habitat-sim/blob/master/.circleci/config.yml#L437

@codecov
Copy link

codecov bot commented May 2, 2020

Codecov Report

Merging #615 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #615   +/-   ##
=======================================
  Coverage   58.69%   58.69%           
=======================================
  Files         136      136           
  Lines        6162     6162           
  Branches       84       84           
=======================================
  Hits         3617     3617           
  Misses       2545     2545           
Flag Coverage Δ
#CPP 53.40% <ø> (ø)
#JavaScript 10.00% <ø> (ø)
#Python 77.24% <ø> (ø)

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 1c8497c...b2fdce8. Read the comment docs.

@dhruvbatra
Copy link
Contributor Author

dhruvbatra commented May 3, 2020

@erikwijmans -- good catch!

I did some more digging, looks like habitat-api docs haven't been built/published for over 7 months (cc: @abhiskk: see pic below for how outdated our api docs are).
image

I added a job for building them. This should be sufficient, but I'm stuck on this error:
facebookresearch/habitat-lab#385

@@ -422,6 +422,32 @@ jobs:
background: true
paths:
- ./habitat-api
- run:
name: Build api documentation
Copy link
Contributor

Choose a reason for hiding this comment

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

We also need to add this (minus the cache saving) to habitat-api to make sure PRs there don't break the doc built for habitat-api, which would then break habitat-sim's CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

conda install -y -c conda-forge pelican
mkdir -p ../build/docs
./build-public.sh
- save_cache:
Copy link
Contributor

Choose a reason for hiding this comment

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

The save_cache block under "Build sim documentation" can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really? Don't we need it for the Update public documentation job?

Copy link
Contributor

Choose a reason for hiding this comment

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

That save is now being done as part of this block (the Build api documentation block).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

conda create -y -n habitat-py38 python=3.8
. activate habitat-py38; #cd habitat-api
conda install -q -y -c conda-forge ninja numpy pytest pytest-cov ccache hypothesis
pip install -r requirements.txt --progress-bar off
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't be needed anymore as facebookresearch/habitat-lab#385 is fixed by #627.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed.

cd docs
conda install -y -c conda-forge doxygen==1.8.16
conda install -y jinja2 pygments docutils
conda install -y -c conda-forge pelican
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pelican is needed for the website only, not for the docs. (So to be also removed in the "Build sim documentation" section where this got copied from.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed.

@erikwijmans
Copy link
Contributor

Probably needs to be merged with master to get the latest m.css changes

@@ -475,6 +486,16 @@ workflows:
- cpp_lint
- js_lint
- install_and_test_ubuntu
nightly:
Copy link
Contributor

Choose a reason for hiding this comment

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

Dhruv, to test the setup you can send commit with commented

#          filters:
#            branches:
#              only: master

Then it will run on your current commit and start updating the doc.

Copy link
Contributor

@mathfac mathfac left a comment

Choose a reason for hiding this comment

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

Dhruv thank you for finalizing this! Great CI skills. As I understand logic will as follow both HSIM & HAPI will have their own build doc step on each PR. And nightly docs will be uploaded to the website from HSIM CI.

CI test need to pass successful and most probably merged with master + try testing without master filter to make sure it works E2E.

@mathfac
Copy link
Contributor

mathfac commented May 12, 2020

Dhruv, you can ssh to CI machine and run experiment commands there, for this you need to enable Github integration:
Screenshot 2020-05-11 20 53 04

Then on failed or successful job of your interest run job with ssh:
image

* master:
  Interactive RigidObject python tutorial page (#611)
  Update README.md
  Separate Scene And Object functionality into different classes (#628)
  Updated m.css submodule. (#627)
  Update agent state set state to be less confusing (#614)
  Direct Navmesh/Meshdata handling; PluginManager instance variable (#623)
  Documentation generator updates (#624)
  Update corrade submodule. (#618)
  Physics primitives (non-colliding) (#622)
  save_nav_mesh python bindings (#619)
  --Renamed GltfMeshData -> GenericMeshData; (#617)
  Update README.md
  Update README.md
@dhruvbatra
Copy link
Contributor Author

Looked into the failing CI. It looks like the same error as previously observed & fixed on h-api. Merging into master to confirm the hypothesis. If tests pass, I am ready to merge this in.

@dhruvbatra
Copy link
Contributor Author

All CI tests are passing. Merging this in. Thanks @mathfac @erikwijmans @mosra

@dhruvbatra dhruvbatra merged commit b97cbd9 into master May 13, 2020
@dhruvbatra dhruvbatra deleted the dhruvbatra-patch-2 branch May 13, 2020 22:12
@mathfac
Copy link
Contributor

mathfac commented May 14, 2020

Thank you! It runs daily now:
image

image

@dhruvbatra
Copy link
Contributor Author

Awesome! Next step: nightly conda builds 😄

Ram81 pushed a commit to Ram81/habitat-web-sim that referenced this pull request Dec 10, 2020
…tra-patch-2

Nightly build for documentation deployment
luoj1 pushed a commit to luoj1/habitat-sim that referenced this pull request Aug 16, 2022
* Update preemption to work with automatic requeueing

* Preemption updates

* Undo

* Remove cast
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants