swancustomenvironments: Remove swan middle layer #421
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rodrigo-sobral
force-pushed
the
master
branch
from
380b3c2 to
2fc2d40
Compare
etejedor
reviewed
Dec 9, 2024
| @@ -229,7 +229,7 @@ <h2 id="modal-title" style="text-align: left;"></h2> | |||
| else { | |||
| outputBox.innerHTML += message_line; | |||
| outputBox.scrollTop = outputBox.scrollHeight; | |||
| if (message_line.includes('ERROR')) { | |||
| if (message_line.toUpperCase().includes('ERROR')) { | |||
There was a problem hiding this comment.
Why do you need the uppercase? Isn't ERROR in lowercase your signal for when something is wrong? Is it to catch other errors that you do not signal?
etejedor
reviewed
Dec 9, 2024
| @@ -1,27 +1,10 @@ | |||
| #!/bin/bash | |||
|
|
|||
| # Create a middle layer for installing ipykernel, putting it apart from the user environment | |||
There was a problem hiding this comment.
The commit message still needs to reflect the fact that venv-pack does not support editable packages, and the middle layer was based on the use of a .pth file, which is seen as a source of editable packages and therefore makes venv-pack fail.
etejedor
reviewed
Dec 9, 2024
| @@ -41,15 +24,28 @@ if [ "${RESOLVED_REQ}" = true ]; then | |||
| # Use the same pip configuration as the Acc-Py default pip | |||
| ACCPY_PIP_CONF="-i $(pip config get global.index-url) --allow-insecure-host $(pip config get global.trusted-host)" | |||
| uv pip install ${ACCPY_PIP_CONF} -r "${REQ_PATH}" 2>&1 | |||
| uv pip install ${ACCPY_PIP_CONF} ${IPYKERNEL} 2>&1 | |||
There was a problem hiding this comment.
Here and below, we need a comment that says:
# Enforce installation of our version of ipykernel and its dependencies
etejedor
reviewed
Dec 9, 2024
| USER_PACKAGES_PATH=$(python3 -c 'import sysconfig; print(sysconfig.get_paths()["purelib"])') | ||
| echo ${SWAN_PACKAGES_PATH} > ${USER_PACKAGES_PATH}/$(basename $SWAN_ENV).pth | ||
| if [ -n "${USE_NXCALS}" ]; then | ||
| # For NXCALS, install the Spark packages and their dependencies, using the according tool for its resolution |
There was a problem hiding this comment.
Suggestion:
# For NXCALS, enforce installation of our Spark extensions and their dependencies at certain versions
etejedor
reviewed
Dec 9, 2024
| @@ -24,14 +17,11 @@ eval "${ACTIVATE_ENV_CMD}" | |||
| _log "Installing packages from ${REQ_PATH}..." | |||
| if [ "${RESOLVED_REQ}" = true ]; then | |||
| uv pip install -r "${REQ_PATH}" 2>&1 | |||
| uv pip install ${IPYKERNEL} 2>&1 | |||
etejedor
reviewed
Dec 9, 2024
rodrigo-sobral
force-pushed
the
master
branch
2 times, most recently
from
1025e6d to
e39bd19
Compare
Keep ipykernel package definition consistent with the remaining extra packages. Due to the fact `venv-pack` (used by nxcals) doesn't support editable packages to be shipped with the environment, and the swan middle layer was based on the use of a .pth file, which is seen as a source of editable packages and therefore makes venv-pack fail. For this reason, the swan middle layer has been removed and the extra packages have been added directly to the environment. In spite of this, the requirements file must not be modified.
…irements Prevents UV errors like `No solution found when resolving dependencies:`
rodrigo-sobral
force-pushed
the
master
branch
from
e39bd19 to
f5db6f6
Compare
etejedor
approved these changes
Dec 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Due to the fact
venv-pack(used by nxcals) doesn't support editable packages to be shipped with the environment, and the swan middle layer was based on the use of a .pth file, which is seen as a source of editable packages and therefore makes venv-pack fail. For this reason, the swan middle layer has been removed and the extra packages have been added directly to the environment. In spite of this, the requirements file must not be modified.