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

Google Bucket hold and docs building fix #6460

Merged
merged 2 commits into from
Nov 2, 2023
Merged

Google Bucket hold and docs building fix #6460

merged 2 commits into from
Nov 2, 2023

Conversation

ssheorey
Copy link
Member

@ssheorey ssheorey commented Oct 31, 2023

  • Put holds on latest artifacts (wheels, tar.gz) in google bucket to prevent deletion.
    Google bucket policy for devel wheels and binaries currently auto-deletes after fixed time (30 days). If no updates are made in that time, docs links become dead. This prevents deletion of latest artifacts.
  • Minor fixes to docs for typos.
  • Prevent overwriting pinned JInja2 version during make install-pip-package. This causes errors during build docs:
    e.g.: https://github.com/isl-org/Open3D/actions/runs/6710379932/job/18235427697#step:8:11005
  • Update build docs instructions

This change is Reviewable

Copy link

update-docs bot commented Oct 31, 2023

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

@ssheorey ssheorey changed the title Bucket hold Google Bucket hold and docs building fix Oct 31, 2023
@ssheorey ssheorey requested a review from errissa October 31, 2023 23:30
during `make install-pip-package`
Update build docs instructions
Copy link
Collaborator

@errissa errissa left a comment

Choose a reason for hiding this comment

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

Reviewed 5 of 6 files at r1, 1 of 2 files at r2, all commit messages.
Reviewable status: 6 of 8 files reviewed, 3 unresolved discussions (waiting on @ssheorey)


docs/getting_started.in.rst line 86 at r2 (raw file):

      - `Python 3.9 <https://storage.googleapis.com/open3d-releases-master/python-wheels/open3d-@OPEN3D_VERSION_FULL@-cp39-cp39-macosx_10_15_x86_64.whl>`__
      - `Python 3.10 <https://storage.googleapis.com/open3d-releases-master/python-wheels/open3d-@OPEN3D_VERSION_FULL@-cp310-cp310-macosx_10_15_x86_64.whl>`__
      - `Python 3.11 <https://storage.googleapis.com/open3d-releases-master/python-wheels/open3d-@OPEN3D_VERSION_FULL@-cp311-cp311-macosx_10_15_universal2.whl>`__

Is this a Python 3.11 thing? When I build under 3.10 I get a wheel with x86_64 or arm64 not universal2


util/ci_utils.sh line 420 at r2 (raw file):

    make -j$NPROC
    bin/GLInfo
    export PYTHONPATH="${PYTHONPATH:+${PYTHONPATH}:}:$PWD/lib/python_package"

I don't understand the purpose of this change.


util/ci_utils.sh line 438 at r2 (raw file):

        -DBUILD_JUPYTER_EXTENSION=OFF \
        ..
    make python-package -j$NPROC

Same as above.

Copy link
Member Author

@ssheorey ssheorey left a comment

Choose a reason for hiding this comment

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

Reviewable status: 6 of 8 files reviewed, 1 unresolved discussion (waiting on @errissa)


docs/getting_started.in.rst line 86 at r2 (raw file):

Previously, errissa (Rene) wrote…

Is this a Python 3.11 thing? When I build under 3.10 I get a wheel with x86_64 or arm64 not universal2

Yes. The current link doesn't work for Python 3.11. Check this link instead:

https://storage.googleapis.com/open3d-releases-master/python-wheels/open3d-0.17.0%2Bad0edd0-cp311-cp311-macosx_10_15_universal2.whl


util/ci_utils.sh line 420 at r2 (raw file):

Previously, errissa (Rene) wrote…

I don't understand the purpose of this change.

Our install dependencies are inconsistent with our docs dependencies. [Our docs building has a lot of patches and needs a serious upgrade]

make pip-install-package -> uses pip and that installs dependencies, specifically:

Collecting Jinja2>=3.1.2 (from Flask<3.1,>=1.0.4->dash>=2.6.0->-r /home/runner/work/Open3D/Open3D/python/requirements.txt (line 2))

However, we havejinja==3.0.3 pinned in docs/requirements.txt since jinja2 v3.1+ breaks our old version of sphinx. This change is a workaround for this breakage.
The long term solution here is to upgrade our sphinx to the latest to prevent these issues. We have had other problems with newer sphinx versions (e.g. search doesn't work) but they have likely been fixed by now.

This workaround avoids using pip and instead changes PYTHONPATH to point to the built version of Open3D.

Copy link
Collaborator

@errissa errissa left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 6 files at r1, 1 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ssheorey)

@ssheorey ssheorey merged commit 6167131 into master Nov 2, 2023
41 of 43 checks passed
@ssheorey ssheorey deleted the ss/bucket-hold branch November 2, 2023 16:03
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.

2 participants