fix(config): set localhost default value for REANA_HOSTNAME #630

Alputer · 2025-01-31T11:46:30Z

This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components. You can refer to other PRs below.

reanahub/reana#867
reanahub/reana-server#717

Closes reanahub/reana#865

codecov · 2025-01-31T11:48:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.77%. Comparing base (bb7b2ff) to head (a02767e).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #630      +/-   ##
==========================================
+ Coverage   73.67%   73.77%   +0.10%     
==========================================
  Files          17       17              
  Lines        1956     1964       +8     
==========================================
+ Hits         1441     1449       +8     
  Misses        515      515

Files with missing lines	Coverage Δ
reana_workflow_controller/config.py	`94.44% <100.00%> (+0.28%)`	⬆️
reana_workflow_controller/consumer.py	`55.76% <100.00%> (ø)`
reana_workflow_controller/rest/utils.py	`82.50% <ø> (ø)`
reana_workflow_controller/rest/workflows.py	`68.97% <ø> (ø)`

…#630) This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components. You can refer to other PRs below. reanahub/reana#867 reanahub/reana-server#717 Closes reanahub/reana#865

This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components. You can refer to other PRs below. reanahub/reana-server#717 reanahub/reana-workflow-controller#630 Closes reanahub#865

…#630) This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components. You can refer to other PRs below. reanahub/reana#867 reanahub/reana-server#717 Closes reanahub/reana#865

…#717) This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components. You can refer to other PRs below. reanahub/reana#867 reanahub/reana-workflow-controller#630 Closes reanahub/reana#865

…#630) This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components. You can refer to other PRs below. reanahub/reana#867 reanahub/reana-server#717 Closes reanahub/reana#865

This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components and introduces REANA_HOSTPORT Helm value. You can refer to other PRs below. reenahub/reana-server#717 reanahub/reana-workflow-controller#630 Closes reanahub#865

tiborsimko · 2025-01-31T15:41:10Z

reana_workflow_controller/config.py

@@ -277,9 +277,12 @@ def _parse_interactive_sessions_environments(env_var):
 REANA_GITLAB_URL = "https://{}".format(REANA_GITLAB_HOST)
 """GitLab API URL"""

-REANA_HOSTNAME = os.getenv("REANA_HOSTNAME", "CHANGE_ME")
+REANA_HOSTNAME = os.getenv("REANA_HOSTNAME", "localhost")
 """REANA URL"""


todo: ‏If we take port out of reana_hostname, we should change the docstring from "REANA URL" to "REANA host name".

polish: ‏For the release notes, for this component the most important news is that Dask dashboard URLs are correct, so something like the following may be appropriate?

fix(dask): use correct REANA host port for Dask service URLs

tiborsimko · 2025-01-31T15:44:13Z

reana_workflow_controller/config.py

 """REANA URL"""

+REANA_HOSTPORT = os.getenv("REANA_HOSTPORT", "30443")
+"""REANA Ingress Port"""


todo: ‏Since "host port" and "ingress" do not have apparent literal connection, let's use a docstring such as
"REANA host name port number". Here but also in Helm value documentation etc.

tiborsimko · 2025-01-31T15:45:56Z

reana_workflow_controller/consumer.py

@@ -196,7 +197,9 @@ def _update_commit_status(workflow, status):
        return
    gitlab_access_token = gitlab_access_token_secret.value_str

-    target_url = f"https://{REANA_HOSTNAME}/api/workflows/{workflow.id_}/logs"
+    target_url = (
+        f"https://{REANA_HOSTNAME}:{REANA_HOSTPORT}/api/workflows/{workflow.id_}/logs"


suggestion: ‏If REANA_HOSTPORT is 443, we don't want to be appending it to HTTPS URLs, because it is more user friendly to write https://reana.cern.ch instead of https://reana.cern.ch:443.

You could introduce a new utility function that will compose URL of the host name and host port information, leaving the port aside if it has the default 443 value. (Or use a function from a Python library.)

…b#630) This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components. You can refer to other PRs below. reanahub/reana#867 reanahub/reana-server#717 Closes reanahub/reana#865

This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components and introduces REANA_HOSTPORT Helm value. You can refer to other PRs below. reenahub/reana-server#717 reanahub/reana-workflow-controller#630 Closes reanahub#865

…b#630) This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components. You can refer to other PRs below. reanahub/reana#867 reanahub/reana-server#717 Closes reanahub/reana#865

This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components and introduces REANA_HOSTPORT Helm value. You can refer to other PRs below. reenahub/reana-server#717 reanahub/reana-workflow-controller#630 Closes reanahub#865

tiborsimko · 2025-02-03T12:38:59Z

reana_workflow_controller/config.py

+REANA_HOSTPORT = os.getenv("REANA_HOSTPORT", "30443")
+"""REANA host name port number."""
+
+BASE_REANA_URL = compose_base_reana_url(REANA_HOSTNAME, REANA_HOSTPORT)


suggestion: ‏We can call the variable REANA_URL, as we did the past. Alternatively, REANA_BASE_URL, or REANA_HOST_URL to make link with host name and host port.

Please note starting with REANA_... rather than BASE_....

(I guess it should be self-understood that it is the base URL of the service, so even REANA_URL should be OK to use. Well, we also have REANA_SERVER_URL, so one could say we are detecting REANA_SERVER_URL from the host name and host port... But rather use something else since that one is usually set by the users. So we may want to keep the distinction of what we detect based on deployment values vs what user sets.)

This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components and introduces REANA_HOSTPORT Helm value. You can refer to other PRs below. reenahub/reana-server#717 reanahub/reana-workflow-controller#630 Closes reanahub#865

…b#630) This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components. You can refer to other PRs below. reanahub/reana#867 reanahub/reana-server#717 Closes reanahub/reana#865

…b#867) This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components and introduces REANA_HOSTPORT Helm value. You can refer to other PRs below. reenahub/reana-server#717 reanahub/reana-workflow-controller#630 Closes reanahub#865

…b#867) This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components and introduces REANA_HOSTPORT Helm value. You can refer to other PRs below. reanahub/reana-server#717 reanahub/reana-workflow-controller#630 Closes reanahub#865

Alputer requested a review from tiborsimko January 31, 2025 11:46

Alputer self-assigned this Jan 31, 2025

Alputer changed the title ~~fix(config): set localhost default value for REANA_HOSTNAME (#630)~~ fix(config): set localhost default value for REANA_HOSTNAME Jan 31, 2025

Alputer mentioned this pull request Jan 31, 2025

fix(config): set localhost default value for REANA_HOSTNAME reanahub/reana-server#717

Merged

Alputer force-pushed the fix-reana-hostname branch from 3d9376d to d4df3a3 Compare January 31, 2025 11:49

Alputer mentioned this pull request Jan 31, 2025

Harmonize default value for REANA_HOSTNAME reanahub/reana#867

Merged

Alputer force-pushed the fix-reana-hostname branch from d4df3a3 to d31a3dd Compare January 31, 2025 12:53

Alputer force-pushed the fix-reana-hostname branch from d31a3dd to fd40874 Compare January 31, 2025 13:43

Alputer force-pushed the fix-reana-hostname branch from fd40874 to 8a8da3a Compare January 31, 2025 13:50

Alputer force-pushed the fix-reana-hostname branch from 8a8da3a to f26b0cc Compare January 31, 2025 14:15

tiborsimko reviewed Jan 31, 2025

View reviewed changes

Alputer force-pushed the fix-reana-hostname branch from f26b0cc to 6819003 Compare January 31, 2025 16:19

Alputer force-pushed the fix-reana-hostname branch from 6819003 to bcda677 Compare January 31, 2025 16:20

Alputer force-pushed the fix-reana-hostname branch from bcda677 to bf8d7e1 Compare January 31, 2025 16:28

Alputer force-pushed the fix-reana-hostname branch from bf8d7e1 to 64dfed1 Compare January 31, 2025 16:35

tiborsimko reviewed Feb 3, 2025

View reviewed changes

fix(dask): use correct REANA host port for Dask service URLs (reanahu…

a02767e

…b#630) This PR is part of harmonizing the treatment of REANA_HOSTNAME accross all REANA components. You can refer to other PRs below. reanahub/reana#867 reanahub/reana-server#717 Closes reanahub/reana#865

Alputer force-pushed the fix-reana-hostname branch from 64dfed1 to a02767e Compare February 3, 2025 14:10

tiborsimko approved these changes Feb 3, 2025

View reviewed changes

tiborsimko merged commit a02767e into reanahub:master Feb 3, 2025
14 checks passed

github-actions bot mentioned this pull request Jan 31, 2025

chore(master): release 0.9.5 #578

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(config): set localhost default value for REANA_HOSTNAME #630

fix(config): set localhost default value for REANA_HOSTNAME #630

Alputer commented Jan 31, 2025 •

edited

Loading

codecov bot commented Jan 31, 2025 •

edited

Loading

tiborsimko Jan 31, 2025

tiborsimko Jan 31, 2025

tiborsimko Jan 31, 2025

tiborsimko Jan 31, 2025

tiborsimko Feb 3, 2025

fix(config): set localhost default value for REANA_HOSTNAME #630

fix(config): set localhost default value for REANA_HOSTNAME #630

Conversation

Alputer commented Jan 31, 2025 • edited Loading

codecov bot commented Jan 31, 2025 • edited Loading

Codecov Report

tiborsimko Jan 31, 2025

Choose a reason for hiding this comment

tiborsimko Jan 31, 2025

Choose a reason for hiding this comment

tiborsimko Jan 31, 2025

Choose a reason for hiding this comment

tiborsimko Jan 31, 2025

Choose a reason for hiding this comment

tiborsimko Feb 3, 2025

Choose a reason for hiding this comment

Alputer commented Jan 31, 2025 •

edited

Loading

codecov bot commented Jan 31, 2025 •

edited

Loading