slurm.el icon indicating copy to clipboard operation
slurm.el copied to clipboard

Cannot connect to remote host using the slurm-remote-host variable

Open aancel opened this issue 3 years ago • 4 comments

Hello,

First thanks for this useful package for slurm management !
I initially tried it some time ago and had issues managing a slurm instance located on a remote machine.
Since then, I've spent some time on the code to try and find the issue I had and I noticed that there are several places where the slurm-remote-host is overriden, and thus breaking the way I expected the package to be working.

To be clearer, what I expected the workflow to be was:

  • Install the package
  • Customize the slurm-remote-host variable to point it to the remote slurm instance
  • Launch the slurm command

Is it how I'm expected to use it ?

Applying the following patch seems to solve the issue I encounter on my side:

diff --git a/slurm-mode.el b/slurm-mode.el
index 71a96ea..e96216c 100644
--- a/slurm-mode.el
+++ b/slurm-mode.el
@@ -257,8 +257,7 @@ Assign it the new value VALUE."
   "Open a `slurm-mode' buffer to manage jobs."
   (interactive)
   (if (file-remote-p default-directory)
-      (setq slurm-remote-host (concat "/ssh:" (file-remote-p default-directory 'host) ":" ";"))
-    (setq slurm-remote-host nil))
+      (setq slurm-remote-host (concat "/ssh:" (file-remote-p default-directory 'host) ":" ";")))
 
   (if (file-remote-p default-directory)
       (switch-to-buffer (get-buffer-create (concat "slurm-" (file-remote-p default-directory 'host))))
@@ -415,7 +414,6 @@ Schedule the following command to be executed after termination of the current o
     (slurm--set :old-position (max (line-number-at-pos) 8))
     (slurm--set :running-commands (slurm--get :command))
     (setq buffer-read-only nil)
-    (setq slurm-remote-host (concat "/ssh:" (nth 1 (split-string (buffer-name) "-")) ":" ";"))
     (erase-buffer)
     (insert (format-time-string "%Y-%m-%d %H:%M:%S\n"))
     (when slurm-display-help

aancel avatar Aug 02 '21 07:08 aancel

Hi,

Thanks for the patch. I will have a further look at this. The current workflow is to call the M-x slurm command on any remote buffer and it will automatically handle the hostname. It will show a slurm-<hostname> buffer which you can use for interacting with the job. All this is now handled through tramp and eshell. Please have a look and let me know if this works for you or not.

Blade6570 avatar Aug 05 '21 11:08 Blade6570

Hello, First, thanks for the feedback !

I have checked with the workflow you suggested and it indeed works without the patch.
Regarding the patch, I am currently not using tramp that much and this modification allowed me to directly get the information without having to explicitly use tramp, only by previously specifying the slurm-remote-host variable. I think that this could also present an advantage for people managing several cluster, where they could define functions such as:

(defun slurm-cluster1 ()
    (let* ((slurm-remote-host "..."))
        (slurm)
    )
)

Moreover, I also have tested the specified workflow with the patch and it seems to be behaving the same way (I haven't use the package that much for now, so I might have missed something breaking somewhere).

I'll leave it up to you whether you would like to accept the patch or not, and of course if it does not break anything

aancel avatar Aug 06 '21 10:08 aancel

Okay, thanks for the explanation. I see your point. Could you please submit this patch as a pull request if it is possible for you? Then I can test that branch and merge it. Otherwise, I will do it from my end at my convenience.

Blade6570 avatar Aug 06 '21 11:08 Blade6570

Great, I just opened the PR then ! (I won't have an internet access next week in case fixes/refactors are required)

aancel avatar Aug 06 '21 14:08 aancel