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
ARROW-14505: [CI][Docs] Exercise documentation builds more frequently #11567
Conversation
|
.github/workflows/docs.yml
Outdated
paths: | ||
- '.github/workflows/docs.yml' | ||
- 'ci/docker/linux-apt-docs.dockerfile' | ||
- 'ci/docker/linux-apt-python-3.dockerfile' | ||
- 'ci/docker/ubuntu-20.10-cpp.dockerfile' | ||
- 'ci/scripts/c_glib_build.sh' | ||
- 'ci/scripts/cpp_build.sh' | ||
- 'ci/scripts/docs_build.sh' | ||
- 'ci/scripts/java_build.sh' | ||
- 'ci/scripts/js_build.sh' | ||
- 'ci/scripts/python_build.sh' | ||
- 'ci/scripts/r_build.sh' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean the workflow only runs on PRs if any of those scripts is edited?
(this is not meant to catch build errors due to changes in the actual docs?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This build will test the complete documentation build only on master and other implementation/binding dependent builds are going to be responsible to test the relevant sections triggered on all PR updates:
- conda-cpp: exercise doxygen
- conda-python: exercise sphinx
- debian-js: exercise js docs
- ubuntu-r: exercise R docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds less fragile to just add the ci
directory here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ubuntu-docs build is too expensive so it's better to restrict for PRs.
It will be executed for all commits on master though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we get any notification when CI runs fail on master?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, even if we would get GHA notifications I assume most of us have filtered those emails out.
We can always inspect the build statuses from https://github.com/apache/arrow/commits/master which helps to identify the breaking commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
docker-compose.yml
Outdated
command: *python-conda-command | ||
command: | ||
["/arrow/ci/scripts/cpp_build.sh /arrow /build && | ||
/arrow/ci/scripts/python_build.sh /arrow /build true && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a point in building docs outside of the dedicated ubuntu-docs
build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The ubuntu-docs
build can take more than 90 minutes so we shouldn't run that on all PRs. Instead we can build language specific docs for PRs and execute the full docs built on the master branch.
bf1f042
to
1ffdf41
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, merging on green
1848bc1
to
1ffdf41
Compare
I've got some cleanups / skips related to DuckDB in #11795. I'm ok merging this + #11795 separately, and if the docs builds still fail after that, I'll skip those examples until we can stabilize that integration (there are a few other tickets, and DuckDB is a bit of a moving target cause we sometimes need to wait for their upstream changes before we can test them reliably) |
OK. I closed ARROW-14896. |
2941ff4
to
7d70b79
Compare
7d70b79
to
896e856
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, merging on green
Benchmark runs are scheduled for baseline = 55ebeb3 and contender = be9a22b. be9a22b is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
A follow-up of apache#11567. https://github.com/ursacomputing/crossbow/runs/4415960154?check_suite_focus=true + mkdir -p /build/docs/c_glib + rsync -a /usr/local/share/gtk-doc/html/ /build/docs/c_glib /arrow/ci/scripts/c_glib_build.sh: line 52: rsync: command not found 127
A follow-up of #11567. https://github.com/ursacomputing/crossbow/runs/4415960154?check_suite_focus=true + mkdir -p /build/docs/c_glib + rsync -a /usr/local/share/gtk-doc/html/ /build/docs/c_glib /arrow/ci/scripts/c_glib_build.sh: line 52: rsync: command not found 127 Closes #11859 from kou/ci-debian-rsync Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
No description provided.