Skip to content
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

fix(rest): detect session status from the pod state #611

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

Alputer
Copy link
Member

@Alputer Alputer commented Nov 18, 2024

@Alputer Alputer requested a review from tiborsimko November 18, 2024 11:02
@Alputer Alputer self-assigned this Nov 18, 2024
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

Attention: Patch coverage is 16.66667% with 15 lines in your changes missing coverage. Please review.

Project coverage is 76.01%. Comparing base (4b0b2ff) to head (907459b).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
reana_workflow_controller/k8s.py 11.11% 8 Missing ⚠️
reana_workflow_controller/rest/workflows.py 22.22% 7 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #611      +/-   ##
==========================================
- Coverage   76.59%   76.01%   -0.58%     
==========================================
  Files          17       17              
  Lines        1846     1864      +18     
==========================================
+ Hits         1414     1417       +3     
- Misses        432      447      +15     
Files with missing lines Coverage Δ
reana_workflow_controller/rest/workflows.py 70.48% <22.22%> (-1.56%) ⬇️
reana_workflow_controller/k8s.py 79.31% <11.11%> (-4.52%) ⬇️

@Alputer Alputer requested a review from mdonadoni November 18, 2024 12:53
Alputer added a commit to Alputer/reana-workflow-controller that referenced this pull request Nov 18, 2024
@Alputer Alputer force-pushed the interactive-session-status-fix branch from a50932b to 49cf295 Compare November 18, 2024 16:24
@Alputer Alputer changed the title fix(sessions): get the session status from the pod state (#611) fix(sessions): get the session status from the pod state Nov 25, 2024
Alputer added a commit to Alputer/reana-workflow-controller that referenced this pull request Jan 15, 2025
@Alputer Alputer force-pushed the interactive-session-status-fix branch from 49cf295 to f88c09a Compare January 15, 2025 14:39
@@ -479,3 +479,16 @@ def delete_dask_dashboard_ingress(cluster_name, workflow_id):
plural="middlewares",
name=f"replacepath-{workflow_id}",
)


def check_pod_status_by_prefix(pod_name_prefix, namespace="default"):
Copy link
Member

Choose a reason for hiding this comment

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

Instead of "default" namespace value, we could use the one we have in r-commons, i.e. REANA_RUNTIME_KUBERNETES_NAMESPACE.

Copy link
Member

Choose a reason for hiding this comment

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

Commit message cosmetics: instead of "get the session status from..." we could say "detect session status from..."

Copy link
Member

Choose a reason for hiding this comment

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

Commit scope cosmetics: since the commit touches component named k8s.py or rest/workflows.py, we should rather use fix(rest) instead of fix(sessions). The scope is usually named after the component or the subcomponent of the source code that the commit touches; it is not named after the functionality. Although we have not been respecting this conventional commits semantics so for so strictly everywhere 😉

@Alputer Alputer force-pushed the interactive-session-status-fix branch from f88c09a to 907459b Compare January 15, 2025 16:55
@tiborsimko tiborsimko changed the title fix(sessions): get the session status from the pod state fix(rest): detect session status from the pod state Jan 16, 2025
@tiborsimko tiborsimko merged commit 907459b into reanahub:master Jan 16, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show Jupyter icon when only when notebook is available
2 participants