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

Clone the contrib repo for every docs dependency install to fix docs build #1464

Merged

Conversation

NathanielRN
Copy link
Contributor

@NathanielRN NathanielRN commented Dec 11, 2020

Description

In the linked issue, it was pointed out that the readTheDocs build is failing with the following error message:

ERROR: Invalid requirement: './opentelemetry-python-contrib/exporter/opentelemetry-exporter-datadog' (from line 9 of docs-requirements.txt)
Hint: It looks like a path. File './opentelemetry-python-contrib/exporter/opentelemetry-exporter-datadog' does not exist.

I've seen this error before and hope that this PR has the solution 😄

Related to: #1352

Solution Explanation

Initially I was hoping to take advantage of pip's --src or --target options to have the repo cloned to the directory I wanted (i.e. ./opentelemtry-python-contrib) but pip does not allow certain options in its requirements.txt file 😕

Next, I read how the readTheDocs configuration file works.

So no luck on readTheDocs.

If we knew the path to the virtual env then we could expect the first -e install and clone the repo at the path <venv_path>/src as in the docs but the build says the venv name is python3.8 -mvirtualenv /home/docs/checkouts/readthedocs.org/user_builds/opentelemetry-python/envs/latest so I don't know what would happen if it went to .... /stable 😕

Finally I decided to just go with cloning the repo every time for these packages. The Contrib repo is 40MB large, and there is a 3GB limit for readTheDocs. We should be fine to merge this, but I think a good follow up would be to remove all the references in docs that need these Contrib packages 😄

But if we merge this at least the build will be fixed 😄

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

I wasn't able to get a local build of readTheDocs working... but hopefully we can try this first and I'll follow up with additional fixes if needed 😄

Does This PR Require a Contrib Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

  • Followed the style guidelines of this project
    - [ ] Changelogs have been updated
    - [ ] Unit tests have been added
    - [ ] Documentation has been updated

@NathanielRN NathanielRN requested review from a team, codeboten and hectorhdzg and removed request for a team December 11, 2020 04:40
docs-requirements.txt Outdated Show resolved Hide resolved
@codeboten codeboten added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Dec 11, 2020
@NathanielRN NathanielRN force-pushed the fix-docs-build-contrib-requirements branch from e59d1ba to 85995c6 Compare December 11, 2020 07:29
@NathanielRN
Copy link
Contributor Author

An ALTERNATIVE more long-term solution to this is #1465

Copy link
Contributor

@ocelotl ocelotl left a comment

Choose a reason for hiding this comment

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

This is a good step forward, thanks @NathanielRN 👍

Copy link
Member

@aabmass aabmass left a comment

Choose a reason for hiding this comment

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

LGTM, but I don't think this closes #1352. That seems to be an issue with git tags and RTD not picking up the changes to stable.

Lets remove the "fixes" and see if it actually fixes after

@NathanielRN
Copy link
Contributor Author

@aabmass Sounds good! I have removed 👍

@NathanielRN NathanielRN changed the title Clone the contrib repo in first contrib install to fix docs build Clone the contrib repo for every docs dependency install to fix docs build Dec 11, 2020
@lzchen lzchen closed this Dec 11, 2020
@lzchen lzchen reopened this Dec 11, 2020
@lzchen lzchen merged commit 6aa09b6 into open-telemetry:master Dec 11, 2020
@NathanielRN NathanielRN deleted the fix-docs-build-contrib-requirements branch December 11, 2020 20:24
codeboten pushed a commit to codeboten/opentelemetry-python that referenced this pull request Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants