README Updates #2
Open
Conversation
This file contains hidden or 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
Reviewer's GuideUpdates project documentation by expanding the README with a full product overview and deployment/usage guide for PA-Cloud, and adds standard GitHub issue templates for bugs and feature requests. Sequence diagram for PA-Cloud user authentication and analysis workflowsequenceDiagram
actor User
participant Browser
participant PublicDNS as Route53DNS
participant JupyterHub as JupyterHubEC2
participant Keycloak
participant InternalDNS
participant DataSHIELD as DataSHIELDServer
User ->> Browser: Enter analytics domain URL
Browser ->> PublicDNS: Resolve analytics hostname
PublicDNS -->> Browser: JupyterHub IP
Browser ->> JupyterHub: HTTPS request to JupyterHub
JupyterHub ->> User: Redirect to identity domain
User ->> Browser: Follow redirect to identity domain
Browser ->> PublicDNS: Resolve identity hostname
PublicDNS -->> Browser: Keycloak IP
Browser ->> Keycloak: HTTPS request to Keycloak login
User ->> Keycloak: Submit credentials via Browser
Keycloak -->> Browser: Authentication success and token
Browser ->> JupyterHub: Return to JupyterHub with token
JupyterHub ->> Keycloak: Validate token
Keycloak -->> JupyterHub: Token valid
JupyterHub -->> User: JupyterHub interface and spawn notebook server
User ->> Browser: Open notebook and run DataSHIELD code
Browser ->> JupyterHub: Execute notebook cell
JupyterHub ->> InternalDNS: Resolve cohortname.pacloud.internal
InternalDNS -->> JupyterHub: DataSHIELD server IP
JupyterHub ->> DataSHIELD: Establish secure connection over VPN
JupyterHub ->> DataSHIELD: Send federated analysis commands
DataSHIELD -->> JupyterHub: Return non-disclosive aggregate results
JupyterHub -->> Browser: Display analysis results to user
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Update issue templates
uniqueg
requested changes
Dec 11, 2025
Contributor
uniqueg
left a comment
uniqueg
left a comment
There was a problem hiding this comment.
Nice, thanks a lot! Just a few easy suggestions
| > This repository is currently configured as per Pacific Analytics' deployment of PA-Cloud. You will need to adjust various values in Terraform if deploying to a different location. | ||
|
|
||
| > [!NOTE] | ||
| > The above is provisional, with exact requirements for a novel environment not fully known. |
Contributor
There was a problem hiding this comment.
Maybe invite people to contribute back their experiences.
uniqueg
approved these changes
Jan 27, 2026
Contributor
|
I forgot about this. I think it's good to go. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 5 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `README.md:24` </location>
<code_context>
+- An AWS account.
+- The AWS CLI installed and logged into the above account (if `aws sts get-caller-identity` succeeds when run, this is met).
+- Sufficient permissions to deploy all resources.
+- Hashicorp's Terraform installed.
+- Access to at least one cohort server, for which:
+ - A server is required, which has:
</code_context>
<issue_to_address>
**suggestion (typo):** Use the correct capitalization for "HashiCorp".
Update this bullet to "HashiCorp's Terraform" to use the correct brand spelling.
```suggestion
- HashiCorp's Terraform installed.
```
</issue_to_address>
### Comment 2
<location> `README.md:44` </location>
<code_context>
+
+2. Add all cohort servers. This can be done in `cohort.tf` by adding further instances of the `cohort` module, ensuring that required additional networking resources are added in `network.tf` as well.
+
+3. Initialise terraform.
```hcl
terraform init
</code_context>
<issue_to_address>
**nitpick (typo):** Capitalize "Terraform" consistently as a proper product name.
To stay consistent with the rest of the document, update this step to "Initialise Terraform."
```suggestion
3. Initialise Terraform.
```
</issue_to_address>
### Comment 3
<location> `README.md:40` </location>
<code_context>
+
+Deployment is handled solely through Terraform.
+
+1. Generate an SSH key. This is to be used to authenticate to on prem servers, and should be added to `authorized_keys` in all cohort servers. These instructions assume this was generated as `./pacloud.key`.
+
+2. Add all cohort servers. This can be done in `cohort.tf` by adding further instances of the `cohort` module, ensuring that required additional networking resources are added in `network.tf` as well.
</code_context>
<issue_to_address>
**suggestion (typo):** Consider hyphenating "on-prem".
Specifically, consider updating "on prem servers" to "on-prem servers" for consistency with common usage.
```suggestion
1. Generate an SSH key. This is to be used to authenticate to on-prem servers, and should be added to `authorized_keys` in all cohort servers. These instructions assume this was generated as `./pacloud.key`.
```
</issue_to_address>
### Comment 4
<location> `README.md:74` </location>
<code_context>
+- `https://identity.propass.pacificanalytics.com` (Keycloak)
+- `https://analytics.propass.pacificanalytics.com` (JupyterHub)
+
+Users must be added to the created realm in Keycloak (`propass` by default) to access Jupyterhub. Keycloak's default administrator credentials can be retrieved from Terraform's state file, or AWS Secrets Manager.
+
+## Analysis
</code_context>
<issue_to_address>
**nitpick (typo):** Align "Jupyterhub" capitalization with earlier uses of "JupyterHub".
Please update "Jupyterhub" to "JupyterHub" here for consistency with earlier usage in the README.
Suggested implementation:
```
Users must be added to the created realm in Keycloak (`propass` by default) to access JupyterHub. Keycloak's default administrator credentials can be retrieved from Terraform's state file, or AWS Secrets Manager.
```
```
Once a user has been created in the `propass` realm in Keycloak, they can log in to JupyterHub and begin performing analysis using DataSHIELD.
```
</issue_to_address>
### Comment 5
<location> `README.md:78` </location>
<code_context>
+
+## Analysis
+
+Once a user has been created in the `propass` realm in Keycloak, they can log in to Jupyterhub and begin performing analysis using DataSHIELD.
+
+When using DataSHIELD, connect using the names specified in the cohort modules, suffixed with `.pacloud.internal`. For example, a cohort called 'example' would set the `name` variable to `example`, then once deployed, connect to it with DataSHIELD via `example.pacloud.internal`.
</code_context>
<issue_to_address>
**nitpick (typo):** Use consistent "JupyterHub" capitalization here as well.
This will match the capitalization used elsewhere in the docs and project.
Suggested implementation:
```
Users must be added to the created realm in Keycloak (`propass` by default) to access JupyterHub. Keycloak's default administrator credentials can be retrieved from Terraform's state file, or AWS Secrets Manager.
```
```
Once a user has been created in the `propass` realm in Keycloak, they can log in to JupyterHub and begin performing analysis using DataSHIELD.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| - An AWS account. | ||
| - The AWS CLI installed and logged into the above account (if `aws sts get-caller-identity` succeeds when run, this is met). | ||
| - Sufficient permissions to deploy all resources. | ||
| - Hashicorp's Terraform installed. |
There was a problem hiding this comment.
suggestion (typo): Use the correct capitalization for "HashiCorp".
Update this bullet to "HashiCorp's Terraform" to use the correct brand spelling.
Suggested change
| - Hashicorp's Terraform installed. | |
| - HashiCorp's Terraform installed. |
|
|
||
| 2. Add all cohort servers. This can be done in `cohort.tf` by adding further instances of the `cohort` module, ensuring that required additional networking resources are added in `network.tf` as well. | ||
|
|
||
| 3. Initialise terraform. |
There was a problem hiding this comment.
nitpick (typo): Capitalize "Terraform" consistently as a proper product name.
To stay consistent with the rest of the document, update this step to "Initialise Terraform."
Suggested change
| 3. Initialise terraform. | |
| 3. Initialise Terraform. |
|
|
||
| Deployment is handled solely through Terraform. | ||
|
|
||
| 1. Generate an SSH key. This is to be used to authenticate to on prem servers, and should be added to `authorized_keys` in all cohort servers. These instructions assume this was generated as `./pacloud.key`. |
There was a problem hiding this comment.
suggestion (typo): Consider hyphenating "on-prem".
Specifically, consider updating "on prem servers" to "on-prem servers" for consistency with common usage.
Suggested change
| 1. Generate an SSH key. This is to be used to authenticate to on prem servers, and should be added to `authorized_keys` in all cohort servers. These instructions assume this was generated as `./pacloud.key`. | |
| 1. Generate an SSH key. This is to be used to authenticate to on-prem servers, and should be added to `authorized_keys` in all cohort servers. These instructions assume this was generated as `./pacloud.key`. |
| - `https://identity.propass.pacificanalytics.com` (Keycloak) | ||
| - `https://analytics.propass.pacificanalytics.com` (JupyterHub) | ||
|
|
||
| Users must be added to the created realm in Keycloak (`propass` by default) to access Jupyterhub. Keycloak's default administrator credentials can be retrieved from Terraform's state file, or AWS Secrets Manager. |
There was a problem hiding this comment.
nitpick (typo): Align "Jupyterhub" capitalization with earlier uses of "JupyterHub".
Please update "Jupyterhub" to "JupyterHub" here for consistency with earlier usage in the README.
Suggested implementation:
Users must be added to the created realm in Keycloak (`propass` by default) to access JupyterHub. Keycloak's default administrator credentials can be retrieved from Terraform's state file, or AWS Secrets Manager.
Once a user has been created in the `propass` realm in Keycloak, they can log in to JupyterHub and begin performing analysis using DataSHIELD.
|
|
||
| ## Analysis | ||
|
|
||
| Once a user has been created in the `propass` realm in Keycloak, they can log in to Jupyterhub and begin performing analysis using DataSHIELD. |
There was a problem hiding this comment.
nitpick (typo): Use consistent "JupyterHub" capitalization here as well.
This will match the capitalization used elsewhere in the docs and project.
Suggested implementation:
Users must be added to the created realm in Keycloak (`propass` by default) to access JupyterHub. Keycloak's default administrator credentials can be retrieved from Terraform's state file, or AWS Secrets Manager.
Once a user has been created in the `propass` realm in Keycloak, they can log in to JupyterHub and begin performing analysis using DataSHIELD.
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.
Adds additional information to
README.mdSummary by Sourcery
Update project documentation and add standard GitHub issue templates.
Documentation:
Chores: