Skip to content

Make ghci parts reusable from cunumeric#796

Merged
sandeepd-nv merged 20 commits intonv-legate:branch-23.09from
sandeepd-nv:make_ghci_parts_reusable_from_cunumeric
Aug 9, 2023
Merged

Make ghci parts reusable from cunumeric#796
sandeepd-nv merged 20 commits intonv-legate:branch-23.09from
sandeepd-nv:make_ghci_parts_reusable_from_cunumeric

Conversation

@sandeepd-nv
Copy link
Contributor

No description provided.

@sandeepd-nv sandeepd-nv added in progress category:task PR is a simple task and will not be included in release notes labels Jul 20, 2023
@sandeepd-nv sandeepd-nv self-assigned this Jul 20, 2023
@sandeepd-nv sandeepd-nv force-pushed the make_ghci_parts_reusable_from_cunumeric branch from eb58290 to f258a33 Compare July 21, 2023 08:41
@sandeepd-nv sandeepd-nv requested review from m3vaz and marcinz July 26, 2023 21:16

env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
BASE_IMAGE: rapidsai/devcontainers:23.06-cpp-cuda11.8-mambaforge-ubuntu22.04
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably update this to 23.08 version of the devcontainer. Not sure if we should do it here right now or in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's do it in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Separate PR would be ideal. Keeping conversation open so we don't forget to do this later.

Copy link
Contributor

Choose a reason for hiding this comment

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

I did this in #725

sleep 10;
. devcontainer-utils-vault-s3-test;
. devcontainer-utils-vault-s3-export 0;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

My understanding of this is lacking. :) Why do we need this?

Copy link
Contributor Author

@sandeepd-nv sandeepd-nv Aug 4, 2023

Choose a reason for hiding this comment

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

devcontainer-utils-vault-s3-test

This tests if AWS credentials are valid.

devcontainer-utils-vault-s3-export

This sets up various SCCACHE_* env vars through ~/.aws/config or ~/.bashrc.

These things are required for enabling SCCACHE for "local repro" where AWS auth happens using a GH personal access token.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also add that the 10 second wait is essential. The credentials that we get are generated on the fly, and need to be uploaded to AWS. We hit authentication issues if we try to race against the upload process.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is also done in the 23.08 devcontainers (and removed in #725).

@sandeepd-nv sandeepd-nv force-pushed the make_ghci_parts_reusable_from_cunumeric branch from caefe9d to b70e06c Compare August 4, 2023 09:25
Copy link
Contributor

@m3vaz m3vaz left a comment

Choose a reason for hiding this comment

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

We should file an issue on github tracking the numpy discrepancy (i.e. the reason why we need the WAR) and reference the issue number in the WAR comment, but otherwise LGTM

@sandeepd-nv sandeepd-nv merged commit 6ce91b1 into nv-legate:branch-23.09 Aug 9, 2023
manopapad pushed a commit that referenced this pull request Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:task PR is a simple task and will not be included in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants