-
Notifications
You must be signed in to change notification settings - Fork 3
WIP: docs(guides): Add new admin guides #32
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
base: main
Are you sure you want to change the base?
Conversation
This PR adds several adminstrator guides regarding how to install the different Elixir Cloud AAI services on a kubernetes/openshift cluster - Add new admin guides - cloud registry - CWL-WES - DRS-filer - proTES - proWES - TESK - TRS-filer - Update Makefile
Reviewer's GuideAdds dedicated admin guides for deploying key ELIXIR Cloud AAI services (Cloud Registry, CWL-WES, DRS-Filer, proTES, proWES, TESK, TRS-Filer, Funnel), wires them into MkDocs navigation, and performs small documentation cleanups plus a more robust virtualenv creation in the Makefile. Sequence diagram for TES task execution via proTES and TESKsequenceDiagram
actor client as Client_or_workflow_engine
participant proTES as proTES
participant mongo as MongoDB_proTES
participant rabbitmq as RabbitMQ_proTES
participant worker as Celery_worker
participant tesk as TESK_backend
client->>proTES: createTask(request_body)
proTES->>mongo: store_incoming_task_metadata()
proTES->>rabbitmq: enqueue_task_for_tracking(task_id)
proTES->>tesk: POST /ga4gh/tes/v1/tasks
tesk-->>proTES: 200 OK with task_id_backend
proTES->>mongo: update_task_with_backend_id(task_id, task_id_backend)
proTES-->>client: 200 OK with task_id
loop async_status_tracking
worker->>rabbitmq: poll_for_task_messages()
rabbitmq-->>worker: task_message(task_id, task_id_backend)
worker->>tesk: GET /ga4gh/tes/v1/tasks/task_id_backend
tesk-->>worker: task_status(state, logs)
worker->>mongo: update_task_status(task_id, state, logs)
end
client->>proTES: getTask(task_id)
proTES->>mongo: fetch_task(task_id)
mongo-->>proTES: task_document(state, logs)
proTES-->>client: 200 OK with task_status
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 16 issues, and left some high level feedback:
- Several of the new admin guides have copy/paste inconsistencies that could confuse readers (e.g.
drsfiler.mdrepeatedly refers to TRS-Filer and the TRS-Filer repo, andprotes.md’s installation section says "repository of proWES"); please align names, repos, and product descriptions with the actual service in each file. - In
cwlwes.md, some references and wording look off (e.g. the Connexion link points to the GA4GH WES repo,[MongoDB]databaseis malformed, and[JSON Web Token]has no target); consider fixing these links/typos so the description is accurate and readable. - For the new shell/YAML examples (e.g. MinIO OIDC exports, Nextflow plugin snippet, Kubernetes env vars), consider adding appropriate code block languages (
bash,yaml,groovy) for better syntax highlighting in the rendered docs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several of the new admin guides have copy/paste inconsistencies that could confuse readers (e.g. `drsfiler.md` repeatedly refers to TRS-Filer and the TRS-Filer repo, and `protes.md`’s installation section says "repository of proWES"); please align names, repos, and product descriptions with the actual service in each file.
- In `cwlwes.md`, some references and wording look off (e.g. the Connexion link points to the GA4GH WES repo, `[MongoDB]database` is malformed, and `[JSON Web Token]` has no target); consider fixing these links/typos so the description is accurate and readable.
- For the new shell/YAML examples (e.g. MinIO OIDC exports, Nextflow plugin snippet, Kubernetes env vars), consider adding appropriate code block languages (`bash`, `yaml`, `groovy`) for better syntax highlighting in the rendered docs.
## Individual Comments
### Comment 1
<location> `Makefile:43-46` </location>
<code_context>
venv:
@echo "\nCreating a virtual environment ++++++++++++++++++++++++++++++++++++++++++++++++\n"
- @python -m venv .venv
+ @if command -v python >/dev/null 2>&1; then \
+ python -m venv .venv; \
+ else \
+ python3 -m venv .venv; \
+ fi
@echo "\nSummary +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n"
</code_context>
<issue_to_address>
**issue (bug_risk):** Prefer `python3` when available to avoid accidentally using a Python 2 `python` binary.
On systems where `python` points to Python 2, this will fail or behave unpredictably. It’s safer to prefer `python3` and only fall back to `python` if `python3` is unavailable.
Example:
```make
@if command -v python3 >/dev/null 2>&1; then \
python3 -m venv .venv; \
elif command -v python >/dev/null 2>&1; then \
python -m venv .venv; \
else \
echo "Python interpreter not found" >&2; exit 1; \
fi
```
</issue_to_address>
### Comment 2
<location> `docs/guides/guide-admin/index.md:84-85` </location>
<code_context>
+- [vsftpd][vsftpd] (FTP)
!!! note "Other storage solutions"
Other S3 and FTP implementations may work but have not being tested.
</code_context>
<issue_to_address>
**issue (typo):** Fix grammar in note about untested storage implementations
Use "have not been tested" instead of "have not being tested".
```suggestion
!!! note "Other storage solutions"
Other S3 and FTP implementations may work but have not been tested.
```
</issue_to_address>
### Comment 3
<location> `docs/guides/guide-admin/services_to_ls_aai.md:71` </location>
<code_context>
# Using LS-Login in MinIO
LS-Login can be activated in MinIO either by using the MinIO console using the OIDC configuration or by setting environmental variables ([MinIO OIDC Documentation](https://min.io/docs/minio/linux/operations/external-iam/configure-openid-external-identity-management.html)).
-- Config URL (MINIO_IDENTITY_OPENID_CONFIG_URL)
</code_context>
<issue_to_address>
**suggestion (typo):** Use "environment variables" rather than "environmental variables"
This term is almost always written as “environment variables” in documentation; please update the wording here.
</issue_to_address>
### Comment 4
<location> `docs/guides/guide-user/index.md:92` </location>
<code_context>
-!!! warning "Under construction"
- More info coming soon...
+You can find an article about NextFlow with GA4GH TES [here](https://techcommunity.microsoft.com/blog/healthcareandlifesciencesblog/introducing-nextflow-with-ga4gh-tes-a-new-era-of-scalable-data-processing-on-azu/4253160)
-## Workflow Execution Service (WES)
</code_context>
<issue_to_address>
**suggestion (typo):** Align "NextFlow" spelling and punctuation
The project name is typically written as “Nextflow” (lowercase “f”). Also add a period at the end of the sentence.
```suggestion
You can find an article about Nextflow with GA4GH TES [here](https://techcommunity.microsoft.com/blog/healthcareandlifesciencesblog/introducing-nextflow-with-ga4gh-tes-a-new-era-of-scalable-data-processing-on-azu/4253160).
```
</issue_to_address>
### Comment 5
<location> `docs/guides/guide-user/index.md:94` </location>
<code_context>
+You can find an article about NextFlow with GA4GH TES [here](https://techcommunity.microsoft.com/blog/healthcareandlifesciencesblog/introducing-nextflow-with-ga4gh-tes-a-new-era-of-scalable-data-processing-on-azu/4253160)
-## Workflow Execution Service (WES)
+To use TES in your Nextflow config, use the plugins `nf-ga4gh`:
+
+```
</code_context>
<issue_to_address>
**nitpick (typo):** Use singular "plugin" since only one plugin is shown
Consider rephrasing to something like: `To use TES in your Nextflow config, use the plugin nf-ga4gh:` for smoother wording.
```suggestion
To use TES in your Nextflow config, use the plugin `nf-ga4gh`:
```
</issue_to_address>
### Comment 6
<location> `docs/guides/guide-user/index.md:128-137` </location>
<code_context>
+The GA4GH [DRS][ga4gh-drs] API provides a standard set of data retrieval methods
</code_context>
<issue_to_address>
**issue (typo):** Fix typos and improve grammar in the DRS description
Please fix the following in the paragraph:
- "environements" → "environments"
- "accross" → "across"
- "retrievess" → "retrieves"
- "endpointsand" → "endpoints and"
Also, the sentence starting with "Some key features like Standardized data access…" is ungrammatical. Consider rephrasing to: "Some key features include standardized data access, which offers a consistent API for retrieving datasets, and cloud-agnostic operation, which works across different cloud infrastructures."
Suggested implementation:
```
It allows researchers to simplify and standardize data retrieval in cloud-based
environments. Some key features include standardized data access, which offers a consistent
API for retrieving datasets, and cloud-agnostic operation, which works across different
cloud infrastructures. Two use cases for the GA4GH DRS:
```
```
to request the dataset. DRS resolves the ID to the actual storage location and
provides signed URLs or access tokens and the pipeline retrieves the data
```
</issue_to_address>
### Comment 7
<location> `docs/guides/guide-user/index.md:150-160` </location>
<code_context>
-!!! warning "Under construction"
- More info coming soon...
+The GA4GH [TRS][ga4gh-trs] API provides a standard mechanism to list, search and
+register tools and worflows across different platforms and cloud environments.
+It supports workflows written in CWL, WDL, Nextflow, Galaxy, Snakemake.
+Here are examples of two use cases:
+
+- Scenario 1: A bioinformatics researcher develops a workflow for variant calling
+ using WDL and Docker containers. They want to share it with collaborators who use
+ different platform. TRS can help, the researcher registers the workflow in a
+ TRS-compliant registry like Dockstore. The collaborators can discover the workflow
+ via TRS API and run it on their platform.
+ TRS will ensure that metadata, versioning, and container are standardized and
+ accessible
+
+- Scenario 2: A hospital’s genomics lab uses an automated pipeline to analyze patient
</code_context>
<issue_to_address>
**issue (typo):** Fix typos and minor grammar issues in the TRS description
Suggested edits:
- "worflows" → "workflows"
- "different platform" → "different platforms"
- Rewrite "TRS can help, the researcher registers…" to avoid the comma splice (e.g., split into two sentences or use a semicolon).
```suggestion
register tools and workflows across different platforms and cloud environments.
It supports workflows written in CWL, WDL, Nextflow, Galaxy, Snakemake.
Here are examples of two use cases:
- Scenario 1: A bioinformatics researcher develops a workflow for variant calling
using WDL and Docker containers. They want to share it with collaborators who use
different platforms. TRS can help: the researcher registers the workflow in a
TRS-compliant registry like Dockstore. The collaborators can discover the workflow
via the TRS API and run it on their platforms.
TRS ensures that metadata, versioning, and containers are standardized and
accessible.
```
</issue_to_address>
### Comment 8
<location> `docs/guides/guide-admin/tesk.md:215-216` </location>
<code_context>
-http://123.123.123.123:8080/ga4gh/tes/v1
-```
-
-You can now test the intallation with the following example call to get a list
-of tasks:
-
</code_context>
<issue_to_address>
**issue (typo):** Fix spelling of "installation"
```suggestion
You can now test the installation with the following example call to get a list
of tasks:
```
</issue_to_address>
### Comment 9
<location> `docs/guides/guide-admin/prowes.md:121-123` </location>
<code_context>
+- `templates/protes/protes-deployment.yaml`
+- `templates/protes/celery-deployment.yaml`
+
+You can use `ReadWriteOnce` if you don't have `StorageClass`
+that supports `RWX`. In that case, a `podAffinity` will be set to have the proTES pods
+running on the same node.
</code_context>
<issue_to_address>
**suggestion (typo):** Add article before "StorageClass"
Change this to "if you don't have a `StorageClass` that supports `RWX`" for grammatical completeness.
```suggestion
You can use `ReadWriteOnce` if you don't have a `StorageClass`
that supports `RWX`. In that case, a `podAffinity` will be set to have the proWES pods
running on the same node.
```
</issue_to_address>
### Comment 10
<location> `docs/guides/guide-admin/protes.md:19` </location>
<code_context>
+balancer workload distribution layer, a public entry point to an enclave of
+independent compute nodes, or a means of collecting telemetry.
+
+When TES requests are received, proTES applies a configured middlewares before
+forwarding the requests to appropriate TES instances in the network. A plugin
+system makes it easy to write and inject middlewares tailored to specific
</code_context>
<issue_to_address>
**issue (typo):** Fix singular/plural in "a configured middlewares"
Use either “applies configured middleware” or “applies configured middlewares”; the current phrase mixes singular and plural.
```suggestion
When TES requests are received, proTES applies configured middleware before
```
</issue_to_address>
### Comment 11
<location> `docs/guides/guide-admin/protes.md:60` </location>
<code_context>
+
+## Installation
+
+You can find a Helm chart in the [GitHub repository](https://github.com/elixir-cloud-aai/proTES/tree/dev/deployment) of proWES
+
+Follow these instructions
</code_context>
<issue_to_address>
**issue (typo):** Correct service name from proWES to proTES
Change "of proWES" to "of proTES" to match the correct service name.
```suggestion
You can find a Helm chart in the [GitHub repository](https://github.com/elixir-cloud-aai/proTES/tree/dev/deployment) of proTES
```
</issue_to_address>
### Comment 12
<location> `docs/guides/guide-admin/protes.md:75` </location>
<code_context>
+
+## Usage
+
+First you must create a namespace in Kubernetes in which to deploy proWES. The
+commands below assume that everything is created in the context of this
+namespace. How the namespace is created depends on the cluster, so we won't
</code_context>
<issue_to_address>
**issue (typo):** Update reference from proWES to proTES
This line should say "deploy proTES" instead of "deploy proWES".
```suggestion
First you must create a namespace in Kubernetes in which to deploy proTES. The
```
</issue_to_address>
### Comment 13
<location> `docs/guides/guide-admin/cloudregistry.md:58` </location>
<code_context>
+
+- `templates/mongo-deploy.yaml`
+
+### Cloud-registry
+
+TRS-Filer is deployed using:
+
+- `templates/cloud-registry-deploy.yaml`
</code_context>
<issue_to_address>
**issue (typo):** Fix copy-paste: Cloud-registry, not TRS-Filer, is deployed here
This line should read: `Cloud-registry is deployed using:`.
```suggestion
Cloud-registry is deployed using:
```
</issue_to_address>
### Comment 14
<location> `docs/guides/guide-admin/drsfiler.md:3-4` </location>
<code_context>
+## Synopsis
+
+Microservice implementing the [Global Alliance for Genomics and
+Health][ga4gh] (GA4GH) [Workflow Execution Service][ga4gh-wes] (WES)
+API specification for the execution of workflows written in the [Common
</code_context>
<issue_to_address>
**issue:** Align DRS-Filer installation text with the DRS service being described
This DRS implementation is described using "TRS-Filer" naming and `trs-filer` links in the installation/usage sections. Either update these to the correct DRS service/repository, or explicitly state that DRS and TRS share the same service to avoid confusion.
</issue_to_address>
### Comment 15
<location> `docs/guides/guide-admin/funnel.md:1-2` </location>
<code_context>
+Follow these instructions if you wish to deploy a TES endpoint in front of your
+HPC/HTC cluster (currently tested with [Slurm][slurm] and [OpenPBS][openpbs].
+
+- Make sure the build dependencies `make` and [Go 1.21+][go-install] are
</code_context>
<issue_to_address>
**issue (typo):** Add missing closing parenthesis in introductory sentence
The parenthesis after "cluster" isn’t closed; add a closing ")" after "[OpenPBS][openpbs]".
```suggestion
Follow these instructions if you wish to deploy a TES endpoint in front of your
HPC/HTC cluster (currently tested with [Slurm][slurm] and [OpenPBS][openpbs]).
```
</issue_to_address>
### Comment 16
<location> `docs/guides/guide-admin/funnel.md:58` </location>
<code_context>
+ [this file][funnel-config-slurm-service] can be used as a template. Set the
+ correct paths to the `funnel` binary and config file.
+
+If successfull Funnel should be listening on port `8080`.
+
+### OpenPBS
</code_context>
<issue_to_address>
**issue (typo):** Fix spelling and add comma in "If successfull Funnel"
Please change to: `If successful, Funnel should be listening on port \\`8080\\`.`
```suggestion
If successful, Funnel should be listening on port `8080`.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Summary
This PR adds several adminstrator guides regarding how to install the different Elixir Cloud AAI services on a kubernetes/openshift cluster
Changes
Summary by Sourcery
Add dedicated administrator guides and navigation entries for deploying ELIXIR Cloud AAI services and improve related user and setup documentation.
Enhancements:
Build:
python3is available.Documentation: