Address PR comments

Author: daniel-thom

api/openapi.yaml

@@ -7187,6 +7187,7 @@ components:
         compute_node_min_time_for_new_jobs_seconds: 300
         worker_startup_script: worker_startup_script
         worker_completion_script: worker_completion_script
+        slurm_config: null
         execution_config: '{"mode":"auto","limit_resources":true,"srun_termination_signal":"TERM@120"}'
       properties:
         id:
@@ -7263,6 +7264,15 @@ components:
           type: boolean
         status_id:
           type: integer
+        slurm_config:
+          deprecated: true
+          description: "Deprecated: Use execution_config instead. JSON-encoded\
+            \ blob of Slurm configuration options for the workflow. May include\
+            \ fields such as limit_resources, use_srun, srun_termination_signal,\
+            \ and enable_cpu_bind. Stored as a JSON string."
+          type:
+          - string
+          - "null"
         execution_config:
           description: "JSON-encoded execution configuration controlling mode\
             \ (direct/slurm/auto) and related settings such as limit_resources,\

julia_client/Torc/src/api/models/model_WorkflowModel.jl

@@ -23,6 +23,7 @@
         use_pending_failed=false,
         enable_ro_crate=false,
         status_id=nothing,
+        slurm_config=nothing,
         execution_config=nothing,
     )
 
@@ -44,6 +45,7 @@
     - use_pending_failed::Bool : Use PendingFailed status for failed jobs (enables AI-assisted recovery)
     - enable_ro_crate::Bool : When true, automatically create RO-Crate entities for workflow files. Input files get entities during initialization; output files get entities on job completion.
     - status_id::Int64
+    - slurm_config::String : Deprecated: Use execution_config instead. JSON-encoded blob of Slurm configuration options for the workflow. May include fields such as limit_resources, use_srun, srun_termination_signal, and enable_cpu_bind. Stored as a JSON string.
     - execution_config::String : JSON-encoded execution configuration controlling mode (direct/slurm/auto) and related settings such as limit_resources, termination_signal, sigterm_lead_seconds, sigkill_headroom_seconds, timeout_exit_code, oom_exit_code, srun_termination_signal, and enable_cpu_bind. The server stores this without interpretation; only the client deserializes it.
 """
 Base.@kwdef mutable struct WorkflowModel <: OpenAPI.APIModel
@@ -65,16 +67,17 @@ Base.@kwdef mutable struct WorkflowModel <: OpenAPI.APIModel
     use_pending_failed::Union{Nothing, Bool} = false
     enable_ro_crate::Union{Nothing, Bool} = false
     status_id::Union{Nothing, Int64} = nothing
+    slurm_config::Union{Nothing, String} = nothing
     execution_config::Union{Nothing, String} = nothing
 
-    function WorkflowModel(id, name, user, description, timestamp, project, metadata, compute_node_expiration_buffer_seconds, compute_node_wait_for_new_jobs_seconds, compute_node_ignore_workflow_completion, compute_node_wait_for_healthy_database_minutes, compute_node_min_time_for_new_jobs_seconds, jobs_sort_method, resource_monitor_config, slurm_defaults, use_pending_failed, enable_ro_crate, status_id, execution_config, )
-        o = new(id, name, user, description, timestamp, project, metadata, compute_node_expiration_buffer_seconds, compute_node_wait_for_new_jobs_seconds, compute_node_ignore_workflow_completion, compute_node_wait_for_healthy_database_minutes, compute_node_min_time_for_new_jobs_seconds, jobs_sort_method, resource_monitor_config, slurm_defaults, use_pending_failed, enable_ro_crate, status_id, execution_config, )
+    function WorkflowModel(id, name, user, description, timestamp, project, metadata, compute_node_expiration_buffer_seconds, compute_node_wait_for_new_jobs_seconds, compute_node_ignore_workflow_completion, compute_node_wait_for_healthy_database_minutes, compute_node_min_time_for_new_jobs_seconds, jobs_sort_method, resource_monitor_config, slurm_defaults, use_pending_failed, enable_ro_crate, status_id, slurm_config, execution_config, )
+        o = new(id, name, user, description, timestamp, project, metadata, compute_node_expiration_buffer_seconds, compute_node_wait_for_new_jobs_seconds, compute_node_ignore_workflow_completion, compute_node_wait_for_healthy_database_minutes, compute_node_min_time_for_new_jobs_seconds, jobs_sort_method, resource_monitor_config, slurm_defaults, use_pending_failed, enable_ro_crate, status_id, slurm_config, execution_config, )
         OpenAPI.validate_properties(o)
         return o
     end
 end # type WorkflowModel
 
-const _property_types_WorkflowModel = Dict{Symbol,String}(Symbol("id")=>"Int64", Symbol("name")=>"String", Symbol("user")=>"String", Symbol("description")=>"String", Symbol("timestamp")=>"String", Symbol("project")=>"String", Symbol("metadata")=>"String", Symbol("compute_node_expiration_buffer_seconds")=>"Int64", Symbol("compute_node_wait_for_new_jobs_seconds")=>"Int64", Symbol("compute_node_ignore_workflow_completion")=>"Bool", Symbol("compute_node_wait_for_healthy_database_minutes")=>"Int64", Symbol("compute_node_min_time_for_new_jobs_seconds")=>"Int64", Symbol("jobs_sort_method")=>"JobsSortMethod", Symbol("resource_monitor_config")=>"String", Symbol("slurm_defaults")=>"String", Symbol("use_pending_failed")=>"Bool", Symbol("enable_ro_crate")=>"Bool", Symbol("status_id")=>"Int64", Symbol("execution_config")=>"String", )
+const _property_types_WorkflowModel = Dict{Symbol,String}(Symbol("id")=>"Int64", Symbol("name")=>"String", Symbol("user")=>"String", Symbol("description")=>"String", Symbol("timestamp")=>"String", Symbol("project")=>"String", Symbol("metadata")=>"String", Symbol("compute_node_expiration_buffer_seconds")=>"Int64", Symbol("compute_node_wait_for_new_jobs_seconds")=>"Int64", Symbol("compute_node_ignore_workflow_completion")=>"Bool", Symbol("compute_node_wait_for_healthy_database_minutes")=>"Int64", Symbol("compute_node_min_time_for_new_jobs_seconds")=>"Int64", Symbol("jobs_sort_method")=>"JobsSortMethod", Symbol("resource_monitor_config")=>"String", Symbol("slurm_defaults")=>"String", Symbol("use_pending_failed")=>"Bool", Symbol("enable_ro_crate")=>"Bool", Symbol("status_id")=>"Int64", Symbol("slurm_config")=>"String", Symbol("execution_config")=>"String", )
 OpenAPI.property_type(::Type{ WorkflowModel }, name::Symbol) = Union{Nothing,eval(Base.Meta.parse(_property_types_WorkflowModel[name]))}
 
 function OpenAPI.check_required(o::WorkflowModel)
@@ -102,6 +105,7 @@ function OpenAPI.validate_properties(o::WorkflowModel)
     OpenAPI.validate_property(WorkflowModel, Symbol("use_pending_failed"), o.use_pending_failed)
     OpenAPI.validate_property(WorkflowModel, Symbol("enable_ro_crate"), o.enable_ro_crate)
     OpenAPI.validate_property(WorkflowModel, Symbol("status_id"), o.status_id)
+    OpenAPI.validate_property(WorkflowModel, Symbol("slurm_config"), o.slurm_config)
     OpenAPI.validate_property(WorkflowModel, Symbol("execution_config"), o.execution_config)
 end
 
@@ -125,4 +129,5 @@ function OpenAPI.validate_property(::Type{ WorkflowModel }, name::Symbol, val)
 
 
 
+
 end

julia_client/julia_client/docs/WorkflowModel.md

@@ -22,6 +22,7 @@ Name | Type | Description | Notes
 **use_pending_failed** | **Bool** | Use PendingFailed status for failed jobs (enables AI-assisted recovery) | [optional] [default to false]
 **enable_ro_crate** | **Bool** | When true, automatically create RO-Crate entities for workflow files. Input files get entities during initialization; output files get entities on job completion. | [optional] [default to false]
 **status_id** | **Int64** |  | [optional] [default to nothing]
+**slurm_config** | **String** | Deprecated: Use execution_config instead. JSON-encoded blob of Slurm configuration options for the workflow. May include fields such as limit_resources, use_srun, srun_termination_signal, and enable_cpu_bind. Stored as a JSON string. | [optional] [default to nothing]
 **execution_config** | **String** | JSON-encoded execution configuration controlling mode (direct/slurm/auto) and related settings such as limit_resources, termination_signal, sigterm_lead_seconds, sigkill_headroom_seconds, timeout_exit_code, oom_exit_code, srun_termination_signal, and enable_cpu_bind. The server stores this without interpretation; only the client deserializes it. | [optional] [default to nothing]
 
 

python_client/src/torc/openapi_client/models/workflow_model.py

@@ -45,8 +45,9 @@ class WorkflowModel(BaseModel):
     use_pending_failed: Optional[StrictBool] = Field(default=False, description="Use PendingFailed status for failed jobs (enables AI-assisted recovery)")
     enable_ro_crate: Optional[StrictBool] = Field(default=False, description="When true, automatically create RO-Crate entities for workflow files. Input files get entities during initialization; output files get entities on job completion.")
     status_id: Optional[StrictInt] = None
+    slurm_config: Optional[StrictStr] = Field(default=None, description="Deprecated: Use execution_config instead. JSON-encoded blob of Slurm configuration options for the workflow. May include fields such as limit_resources, use_srun, srun_termination_signal, and enable_cpu_bind. Stored as a JSON string.")
     execution_config: Optional[StrictStr] = Field(default=None, description="JSON-encoded execution configuration controlling mode (direct/slurm/auto) and related settings such as limit_resources, termination_signal, sigterm_lead_seconds, sigkill_headroom_seconds, timeout_exit_code, oom_exit_code, srun_termination_signal, and enable_cpu_bind. The server stores this without interpretation; only the client deserializes it.")
-    __properties: ClassVar[List[str]] = ["id", "name", "user", "description", "timestamp", "project", "metadata", "compute_node_expiration_buffer_seconds", "compute_node_wait_for_new_jobs_seconds", "compute_node_ignore_workflow_completion", "compute_node_wait_for_healthy_database_minutes", "compute_node_min_time_for_new_jobs_seconds", "jobs_sort_method", "resource_monitor_config", "slurm_defaults", "use_pending_failed", "enable_ro_crate", "status_id", "execution_config"]
+    __properties: ClassVar[List[str]] = ["id", "name", "user", "description", "timestamp", "project", "metadata", "compute_node_expiration_buffer_seconds", "compute_node_wait_for_new_jobs_seconds", "compute_node_ignore_workflow_completion", "compute_node_wait_for_healthy_database_minutes", "compute_node_min_time_for_new_jobs_seconds", "jobs_sort_method", "resource_monitor_config", "slurm_defaults", "use_pending_failed", "enable_ro_crate", "status_id", "slurm_config", "execution_config"]
 
     model_config = ConfigDict(
         populate_by_name=True,
@@ -87,6 +88,11 @@ def to_dict(self) -> Dict[str, Any]:
             exclude=excluded_fields,
             exclude_none=True,
         )
+        # set to None if slurm_config (nullable) is None
+        # and model_fields_set contains the field
+        if self.slurm_config is None and "slurm_config" in self.model_fields_set:
+            _dict['slurm_config'] = None
+
         # set to None if execution_config (nullable) is None
         # and model_fields_set contains the field
         if self.execution_config is None and "execution_config" in self.model_fields_set:
@@ -122,6 +128,7 @@ def from_dict(cls, obj: Optional[Dict[str, Any]]) -> Optional[Self]:
             "use_pending_failed": obj.get("use_pending_failed") if obj.get("use_pending_failed") is not None else False,
             "enable_ro_crate": obj.get("enable_ro_crate") if obj.get("enable_ro_crate") is not None else False,
             "status_id": obj.get("status_id"),
+            "slurm_config": obj.get("slurm_config"),
             "execution_config": obj.get("execution_config")
         })
         return _obj

src/client/async_cli_command.rs

@@ -117,15 +117,15 @@ fn build_srun_command(params: &SrunParams) -> Result<Command, String> {
     // Floor of 1 minute because --time=0 means unlimited in Slurm.
     if let Some(end) = params.end_time {
         let remaining_secs = (end - Utc::now()).num_seconds();
-        if remaining_secs <= params.sigkill_headroom_seconds {
+        let usable_secs = remaining_secs - params.sigkill_headroom_seconds;
+        if usable_secs < 60 {
             return Err(format!(
                 "Refusing to launch srun step for job {}: only {}s remaining \
-                 (need at least {}s sigkill headroom)",
-                params.job_id, remaining_secs, params.sigkill_headroom_seconds
+                 ({}s usable after {}s sigkill headroom, need at least 60s)",
+                params.job_id, remaining_secs, usable_secs, params.sigkill_headroom_seconds
             ));
         }
-        let headroom_minutes = (params.sigkill_headroom_seconds + 59) / 60;
-        let remaining_minutes = ((remaining_secs / 60) - headroom_minutes).max(1);
+        let remaining_minutes = usable_secs / 60;
         srun.arg(format!("--time={}", remaining_minutes));
     }
 
@@ -343,6 +343,7 @@ impl AsyncCliCommand {
                         resource_requirements
                             .and_then(|rr| memory_string_to_bytes(&rr.memory).ok())
                             .map(|b| b as u64)
+                            .filter(|&b| b > 0)
                     } else {
                         None
                     };

src/server/api/workflows.rs

@@ -285,6 +285,7 @@ impl WorkflowsApiImpl {
                 ,w.project
                 ,w.metadata
                 ,w.status_id
+                ,w.slurm_config
                 ,w.execution_config
             FROM workflow w
             INNER JOIN workflow_status ws ON w.status_id = ws.id
@@ -311,6 +312,7 @@ impl WorkflowsApiImpl {
                 ,project
                 ,metadata
                 ,status_id
+                ,slurm_config
                 ,execution_config
             FROM workflow
             "
@@ -474,7 +476,10 @@ impl WorkflowsApiImpl {
                 project: record.get("project"),
                 metadata: record.get("metadata"),
                 status_id: Some(record.get("status_id")),
-                slurm_config: None,
+                slurm_config: record
+                    .try_get::<Option<String>, _>("slurm_config")
+                    .ok()
+                    .flatten(),
                 execution_config: record
                     .try_get::<Option<String>, _>("execution_config")
                     .ok()
@@ -650,9 +655,10 @@ where
                 project,
                 metadata,
                 status_id,
+                slurm_config,
                 execution_config
             )
-            VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18)
+            VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19)
             RETURNING rowid
             "#,
             body.name,
@@ -672,6 +678,7 @@ where
             body.project,
             body.metadata,
             status_result[0].id,
+            body.slurm_config,
             body.execution_config,
         )
         .fetch_all(&mut *tx)
@@ -947,7 +954,7 @@ where
                     project: row.project,
                     metadata: row.metadata,
                     status_id: Some(row.status_id),
-                    slurm_config: None,
+                    slurm_config: row.slurm_config,
                     execution_config: row.execution_config,
                 },
             )),
@@ -1256,8 +1263,9 @@ where
                 enable_ro_crate = COALESCE($10, enable_ro_crate),
                 project = COALESCE($11, project),
                 metadata = COALESCE($12, metadata),
-                execution_config = COALESCE($13, execution_config)
-            WHERE id = $14
+                slurm_config = COALESCE($13, slurm_config),
+                execution_config = COALESCE($14, execution_config)
+            WHERE id = $15
             "#,
             body.name,
             body.description,
@@ -1271,6 +1279,7 @@ where
             enable_ro_crate_int,
             body.project,
             body.metadata,
+            body.slurm_config,
             body.execution_config,
             id
         )

tests/test_execution_config.rs

@@ -200,6 +200,8 @@ fn test_effective_mode_slurm() {
 #[test]
 #[serial]
 fn test_effective_mode_auto_without_slurm_env() {
+    // Save original value
+    let original = std::env::var("SLURM_JOB_ID").ok();
     // Ensure SLURM_JOB_ID is not set
     // SAFETY: Using serial_test to prevent concurrent access to env vars
     unsafe {
@@ -212,11 +214,20 @@ fn test_effective_mode_auto_without_slurm_env() {
     };
     assert_eq!(config.effective_mode(), ExecutionMode::Direct);
     assert!(!config.use_srun());
+
+    // Restore original value
+    if let Some(val) = original {
+        unsafe {
+            std::env::set_var("SLURM_JOB_ID", val);
+        }
+    }
 }
 
 #[test]
 #[serial]
 fn test_effective_mode_auto_with_slurm_env() {
+    // Save original value
+    let original = std::env::var("SLURM_JOB_ID").ok();
     // Set SLURM_JOB_ID temporarily
     // SAFETY: Using serial_test to prevent concurrent access to env vars
     unsafe {
@@ -230,9 +241,13 @@ fn test_effective_mode_auto_with_slurm_env() {
     assert_eq!(config.effective_mode(), ExecutionMode::Slurm);
     assert!(config.use_srun());
 
-    // Clean up
+    // Restore original value
     unsafe {
-        std::env::remove_var("SLURM_JOB_ID");
+        if let Some(val) = original {
+            std::env::set_var("SLURM_JOB_ID", val);
+        } else {
+            std::env::remove_var("SLURM_JOB_ID");
+        }
     }
 }
 

tests/test_srun_args.rs

@@ -433,32 +433,46 @@ fn test_srun_with_end_time() {
 
 #[test]
 #[serial]
-fn test_srun_with_end_time_minimum_one_minute() {
+fn test_srun_with_end_time_insufficient_time_rejected() {
     let temp_dir = TempDir::new().unwrap();
-    let args_log = setup_srun_env(&temp_dir);
+    let _args_log = setup_srun_env(&temp_dir);
     let rr = make_rr("compute", 2, "4g", 1);
 
-    // Set end_time 10 seconds from now (should clamp to minimum 1 minute)
+    // Set end_time 10 seconds from now — with 60s default headroom, usable time
+    // is negative, so the launch should be refused.
     let end_time = chrono::Utc::now() + chrono::Duration::seconds(10);
 
-    let args = run_and_capture_srun_args(
-        &temp_dir,
-        &args_log,
+    let job = make_job(1, "echo hello");
+    let mut cmd = AsyncCliCommand::new(job);
+
+    let result = cmd.start(
+        temp_dir.path(),
+        1,
+        1,
+        1,
+        None,
+        "http://localhost:8080/torc-service/v1",
         Some(&rr),
         true,
         ExecutionMode::Slurm,
         false,
         Some(end_time),
         None,
-    )
-    .expect("srun should have been invoked");
+        60,
+        None,
+    );
 
     cleanup_srun_env();
 
     assert!(
-        args.contains("--time=1"),
-        "Should clamp to --time=1 for near-expired end_time: {}",
-        args
+        result.is_err(),
+        "Should refuse to launch srun step when insufficient time remains"
+    );
+    let err_msg = result.unwrap_err().to_string();
+    assert!(
+        err_msg.contains("Refusing to launch"),
+        "Error should mention refusing to launch: {}",
+        err_msg
     );
 }