Skip to content

Paver (Part 1) -- Attempt 2 - #2615

Merged
wedaly merged 1 commit into
masterfrom
will/paver-attempt-2
Feb 24, 2014
Merged

Paver (Part 1) -- Attempt 2#2615
wedaly merged 1 commit into
masterfrom
will/paver-attempt-2

Conversation

@wedaly

@wedaly wedaly commented Feb 15, 2014

Copy link
Copy Markdown
Contributor

When I merged the Paver PR into master, Jenkins started failing in bizarre ways. It turns out that:

  1. When we run the test suite, we pip install -e . (here: https://github.com/edx/edx-platform/blob/master/requirements/edx/local.txt#L2)
  2. This adds the path to the current repo directory into sys.path.
  3. However, in Jenkins, there are multiple copies of the repository (for each job, and each sub-job in a multiconfig job).
  4. Therefore, when we first use Python, sys.path may contain the path to another repo.
  5. Because we run paver before re-running pip install on local requirements, the pavelib/assets.py in another copy of the repository was taking precedence over the local copy.
  6. The other copy of pavelib/assets.py constructed a path to the wrong repository, where the prereqs cache indicated that coffee had already been installed (in "node_modules/.bin")
  7. Since coffee had NOT, in fact, been installed in the current repository, coffescript compilation would fail.

Whew!

The right solution is probably to have Jenkins better isolate virtualenvs between jobs. But that would be unacceptably slow, which is why we don't do it.

The solution I've adopted here is to insert the current working directory as the first item in sys.path before loading the pavement modules. This ensures that Paver thinks it's in the right directory.

The only difference between this and the original PR is the sys.path manipulation in pavement.py, which includes a detailed comment.

@yarko

yarko commented Feb 17, 2014

Copy link
Copy Markdown
Contributor

@wedaly - ah; makes sense. Soon as you merge this, I'll rebase my https://github.com/yarko/edx-platform/tree/yarko/pr-2520-default-variant and PR it to add fixes for custom theme behavior (similar to what #2387 fixes w/ rake).

Comment thread pavement.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@wedaly I think you want to sys.path.insert(1,foo)?
That seems to be the standard way for prepending a dir to the python path.
See http://docs.python.org/2/library/sys.html#sys.path

Restore reverted changes from paver part 1
@jzoldak

jzoldak commented Feb 20, 2014

Copy link
Copy Markdown
Contributor

@wedaly, @Cale had this on his branch and noticed that the list of python prereqs wasn't sufficient (it didn't include github.txt or private.txt). Let's investigate on the next round.

@wedaly

wedaly commented Feb 20, 2014

Copy link
Copy Markdown
Contributor Author

@jzoldak "github.txt" is included implicitly in "base.txt" (by way of "repo.txt").

I'm not sure why "private.txt" is necessary -- why wouldn't someone just install extra requirements in the virtualenv?

wedaly pushed a commit that referenced this pull request Feb 24, 2014
@wedaly
wedaly merged commit 38d403f into master Feb 24, 2014
@wedaly
wedaly deleted the will/paver-attempt-2 branch February 24, 2014 12:51
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Nov 12, 2018
kdmccormick pushed a commit to kdmccormick/openedx-platform that referenced this pull request Apr 9, 2025
* feat: add admin action to import course to library

* refactor: [AXM-1807] finish actions, refactor validation & purpose uniq

---------

Co-authored-by: Andrii <andrii.hantkovskyi@raccoongang.com>
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