Skip to content

Commit

Permalink
Use diff join for workflow links (#873)
Browse files Browse the repository at this point in the history
**Pull Request Checklist**
- [x] Fixes #872
- [x] Tests added
- [ ] Documentation/examples added
- [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR
title

The `get_workflow_link` API assembles the link using a simple string
join. However, that can result in double `//` in the middle of a URL,
which is not valid. This adds the use of `os.path.join` so that double
`//` are not a risk anymore.

Signed-off-by: Flaviu Vadan <[email protected]>
  • Loading branch information
flaviuvadan authored Nov 23, 2023
1 parent e3a6cc9 commit 9940b48
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 4 deletions.
5 changes: 3 additions & 2 deletions scripts/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ def get_service_def() -> str:
# [DO NOT MODIFY] Auto-generated by `hera.scripts.service.py`
from urllib.parse import urljoin
import requests
import os
from hera.{module}.models import {imports}
from hera.shared import global_config
from hera.exceptions import exception_from_server_response
Expand Down Expand Up @@ -434,7 +435,7 @@ def add_get_workflow_link(service_def: str) -> str:
+ """
def get_workflow_link(self, name: str) -> str:
\"\"\"Returns the workflow link for the given workflow name.\"\"\"
return f\"{self.host}/workflows/{self.namespace}/{name}?tab=workflow\"
return os.path.join(self.host, f"workflows/{self.namespace}/{name}?tab=workflow")
"""
)

Expand All @@ -446,7 +447,7 @@ def add_get_cron_workflow_link(service_def: str) -> str:
+ """
def get_cron_workflow_link(self, name: str) -> str:
\"\"\"Returns the link for the given cron workflow name.\"\"\"
return f\"{self.host}/cron-workflows/{self.namespace}/{name}\"
return os.path.join(self.host, f"cron-workflows/{self.namespace}/{name}")
"""
)

Expand Down
5 changes: 3 additions & 2 deletions src/hera/workflows/service.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Interact with the REST service."""
# [DO NOT MODIFY] Auto-generated by `hera.scripts.service.py`
import os
from typing import Optional, cast
from urllib.parse import urljoin

Expand Down Expand Up @@ -1303,11 +1304,11 @@ def get_input_artifact(self, name: str, node_id: str, artifact_name: str, namesp

def get_workflow_link(self, name: str) -> str:
"""Returns the workflow link for the given workflow name."""
return f"{self.host}/workflows/{self.namespace}/{name}?tab=workflow"
return os.path.join(self.host, f"workflows/{self.namespace}/{name}?tab=workflow")

def get_cron_workflow_link(self, name: str) -> str:
"""Returns the link for the given cron workflow name."""
return f"{self.host}/cron-workflows/{self.namespace}/{name}"
return os.path.join(self.host, f"cron-workflows/{self.namespace}/{name}")


__all__ = ["WorkflowsService"]
7 changes: 7 additions & 0 deletions tests/test_unit/test_cron_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,10 @@ def test_returns_expected_workflow_link():
workflows_service=WorkflowsService(host="hera.test", namespace="my-namespace"),
)
assert w.get_workflow_link() == "hera.test/cron-workflows/my-namespace/test"

w = CronWorkflow(
name="test",
schedule="* * * * *",
workflows_service=WorkflowsService(host="https://localhost:8443/argo/", namespace="my-namespace"),
)
assert w.get_workflow_link() == "https://localhost:8443/argo/cron-workflows/my-namespace/test"
5 changes: 5 additions & 0 deletions tests/test_unit/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,8 @@ def test_returns_expected_workflow_link():

w = Workflow(name="test", workflows_service=WorkflowsService(host="hera.test", namespace="my-namespace"))
assert w.get_workflow_link() == "hera.test/workflows/my-namespace/test?tab=workflow"

w = Workflow(
name="test", workflows_service=WorkflowsService(host="https://localhost:8443/argo/", namespace="my-namespace")
)
assert w.get_workflow_link() == "https://localhost:8443/argo/workflows/my-namespace/test?tab=workflow"

0 comments on commit 9940b48

Please sign in to comment.