[ENH]: Specify which shell to use when using junifer queue #273

2024-03-20T13:49:02Z

synchon commented

2024-03-20 13:49:02 +00:00

(Migrated from github.com)

Are you requiring a new dataset or marker?

I understand this is not a marker or dataset request

Which feature do you want to include?

I would like to be able to specify the shell that junifer uses to submit jobs. For some reason when using bash I could not get jobs to work. When using zsh by manually changing it, it seemed to work (maybe something from with my bash environment, maybe magic or maybe coincidence). Either way I think this could be a desirable feature. What do you think?

How do you imagine this integrated in junifer?

as a param that prints different text

Do you have a sample code that implements this outside of junifer?

No response

Anything else to say?

No response

### Are you requiring a new dataset or marker? - [X] I understand this is not a marker or dataset request ### Which feature do you want to include? I would like to be able to specify the shell that junifer uses to submit jobs. For some reason when using bash I could not get jobs to work. When using zsh by manually changing it, it seemed to work (maybe something from with my bash environment, maybe magic or maybe coincidence). Either way I think this could be a desirable feature. What do you think? ### How do you imagine this integrated in junifer? as a param that prints different text ### Do you have a sample code that implements this outside of junifer? _No response_ ### Anything else to say? _No response_

synchon commented

2023-11-02 12:54:11 +00:00

(Migrated from github.com)

This could be quite useful in my opinion.

LeSasse (Migrated from github.com) reviewed

2024-03-20 13:49:02 +00:00

synchon commented

2024-03-20 13:52:02 +00:00

(Migrated from github.com)

@LeSasse Can you check if the branch solves your issue?

github-actions[bot] commented

2024-03-20 13:55:20 +00:00

(Migrated from github.com)

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-04-09 12:13 UTC

[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.4.7 :---: Preview removed because the pull request was closed. 2024-04-09 12:13 UTC

LeSasse (Migrated from github.com) reviewed

2024-03-20 13:59:48 +00:00

junifer/api/queue_context/htcondor_adapter.py

would it be better to do /usr/bin/env {shell}?

would it be better to do `/usr/bin/env {shell}`?

junifer/api/res/run_conda.zsh

					
				@ -0,0 +1,23 @@

				#!/usr/bin/env zsh

would it be viable to have the res as a plain txt containing a text with variable placeholders that get replaced by fill values for a specific shell or are the scripts for the different shell vastly different? just wondering if its desirable avoiding 1 script per shell.

would it be viable to have the `res` as a plain `txt` containing a text with variable placeholders that get replaced by fill values for a specific shell or are the scripts for the different shell vastly different? just wondering if its desirable avoiding 1 script per shell.

junifer/api/res/run_venv.zsh

					
				@ -0,0 +1,22 @@

				#!/usr/bin/env zsh

nice

synchon (Migrated from github.com) reviewed

2024-03-20 14:02:30 +00:00

junifer/api/queue_context/htcondor_adapter.py

The standard advice that I see in the wild is to use #!/bin/<shell>. Is there a particular reason for your suggestion?

The standard advice that I see in the wild is to use `#!/bin/<shell>`. Is there a particular reason for your suggestion?

synchon (Migrated from github.com) reviewed

2024-03-20 14:03:38 +00:00

junifer/api/res/run_conda.zsh

					
				@ -0,0 +1,23 @@

				#!/usr/bin/env zsh

We would need the file to be created on-demand for that and not have it as a file distributed with the package. I wanted a single file but for simplicity made it separate.

LeSasse (Migrated from github.com) reviewed

2024-03-20 14:08:39 +00:00

junifer/api/queue_context/htcondor_adapter.py

Running a command through /usr/bin/env has the benefit of looking for whatever the default version of the program is in your current environment.

This way, you don't have to look for it in a specific place on the system, as those paths may be in different locations on different systems. As long as it's in your path, it will find it.

But at the end of the day it isnt really going to make a difference.

Running a command through /usr/bin/env has the benefit of looking for whatever the default version of the program is in your current environment. This way, you don't have to look for it in a specific place on the system, as those paths may be in different locations on different systems. As long as it's in your path, it will find it. But at the end of the day it isnt really going to make a difference.

synchon (Migrated from github.com) reviewed

2024-03-20 14:26:06 +00:00

junifer/api/queue_context/htcondor_adapter.py

That's a fair argument if you want to have control over the shell used. Would you say we extend it to run_{conda,venv}.{bash,zsh} as well?

That's a fair argument if you want to have control over the shell used. Would you say we extend it to `run_{conda,venv}.{bash,zsh}` as well?

codecov[bot] commented

2024-03-20 14:43:17 +00:00

(Migrated from github.com)

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (b0c0a61) to head (de69448).

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #273   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            1         1           
=========================================
  Hits             1         1

Flag	Coverage Δ
docs	`100.00% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

## [Codecov](https://app.codecov.io/gh/juaml/junifer/pull/273?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report All modified and coverable lines are covered by tests :white_check_mark: > Project coverage is 100.00%. Comparing base [(`b0c0a61`)](https://app.codecov.io/gh/juaml/junifer/commit/b0c0a6117c98cada25adcc8729151b5f8e69d76c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) to head [(`de69448`)](https://app.codecov.io/gh/juaml/junifer/pull/273?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml). <details><summary>Additional details and impacted files</summary> [![Impacted file tree graph](https://app.codecov.io/gh/juaml/junifer/pull/273/graphs/tree.svg?width=650&height=150&src=pr&token=5H21JuZXMw&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml)](https://app.codecov.io/gh/juaml/junifer/pull/273?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #273 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 1 1 Lines 1 1 ========================================= Hits 1 1 ``` | [Flag](https://app.codecov.io/gh/juaml/junifer/pull/273/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [docs](https://app.codecov.io/gh/juaml/junifer/pull/273/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `100.00% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#carryforward-flags-in-the-pull-request-comment) to find out more. </details>

LeSasse (Migrated from github.com) reviewed

2024-03-21 06:40:59 +00:00

junifer/api/queue_context/htcondor_adapter.py

Yes, i think so, but didnt wanna duplicate the comment.

LeSasse commented

2024-03-21 06:41:51 +00:00

(Migrated from github.com)

@LeSasse Can you check if the branch solves your issue?

Will check it

> @LeSasse Can you check if the branch solves your issue? Will check it

fraimondo (Migrated from github.com) approved these changes

2024-03-21 07:29:34 +00:00

LeSasse commented

2024-04-05 09:51:13 +00:00

(Migrated from github.com)

@LeSasse Can you check if the branch solves your issue?

I think this branch needs a rebase: the dag files still use single quotes and submitting therefore fails.

> @LeSasse Can you check if the branch solves your issue? I think this branch needs a rebase: the dag files still use single quotes and submitting therefore fails.

synchon commented

2024-04-05 10:07:56 +00:00

(Migrated from github.com)

@LeSasse Can you check if the branch solves your issue?

I think this branch needs a rebase: the dag files still use single quotes and submitting therefore fails.

I see that the quotes are proper in dag, can you try rebasing your local branch?

> > @LeSasse Can you check if the branch solves your issue? > > I think this branch needs a rebase: the dag files still use single quotes and submitting therefore fails. I see that the quotes are proper in `dag`, can you try rebasing your local branch?

LeSasse commented

2024-04-05 10:23:30 +00:00

(Migrated from github.com)

@LeSasse Can you check if the branch solves your issue?

I think this branch needs a rebase: the dag files still use single quotes and submitting therefore fails.

I see that the quotes are proper in dag, can you try rebasing your local branch?

thats quite odd because i actually cloned and checked out this branch just minutes before that comment, and after i rebased it locally it worked, but before, no.

Ah no, this was supposed to go on the other PR #161 so thats the one that would need rebasing

> > > @LeSasse Can you check if the branch solves your issue? > > > > > > I think this branch needs a rebase: the dag files still use single quotes and submitting therefore fails. > > I see that the quotes are proper in `dag`, can you try rebasing your local branch? thats quite odd because i actually cloned and checked out this branch just minutes before that comment, and after i rebased it locally it worked, but before, no. Ah no, this was supposed to go on the other PR #161 so thats the one that would need rebasing

LeSasse commented

2024-04-09 11:48:33 +00:00

(Migrated from github.com)

Tested this now and it works.

🚀 1

Sign in to join this conversation.

No reviewers

LeSasse

fraimondo