Bug#850891: Additional patch, to cover slurmdbd's duplicate code

[Gennaro Oliva]

tags 850891 + wontfix

Hi Rémi and Karl,
since the discussion about this possible feature is over I'm tagging 
this old bug with wontfix.
Thanks for your contribution
-- 
Gennaro Oliva

Bug#850891: Additional patch, to cover slurmdbd's duplicate code

[Gennaro Oliva]

Hi Rémi and Karl,

On Wed, Jan 11, 2017 at 09:23:14PM +, Karl Kornel wrote:
> If I would play devil’s advocate, then I would say this: “An action
> like this seems inappropriate to place into a package that does not
> clearly indicate why it is needed.  Instead, you should create a
> slurm-common package, and have the user creation happen there.”

This is the reason because we didn't include this change into
Debian: I understand that repeating code is not so smart, but decoupling
the action of creating user from the packages that actually need it, in
my opinion is worst since it hides the reasons behind the user creation.

> I would be OK with that: I am fine putting together a patch to create
> a “slurm-common” package that does the user creation.  Some common
> documentation could also be moved to that package.  But, I did not
> know if that was appropriate, so I submitted the simpler change first!

I think that a package for the user creation is a bit excessive.
Best regards,
-- 
Gennaro Oliva

Bug#850891: Additional patch, to cover slurmdbd's duplicate code

[Karl Kornel]

Hi Rémi, (I hope the é comes through the email properly!)

If I would play devil’s advocate, then I would say this: “An action like this 
seems inappropriate to place into a package that does not clearly indicate why 
it is needed.  Instead, you should create a slurm-common package, and have the 
user creation happen there.”

I would be OK with that: I am fine putting together a patch to create a 
“slurm-common” package that does the user creation.  Some common documentation 
could also be moved to that package.  But, I did not know if that was 
appropriate, so I submitted the simpler change first!

~ Karl

On 1/11/17, 1:21 AM, "Rémi Palancher"  wrote:

FWIW, we took the same approach in our EDF-specific packages:

https://github.com/edf-hpc/slurm-llnl

It works fine for us so far!

I just don't remember why we didn't propose the patch back into Debian 
official packages though :(

Best,
Rémi

Bug#850891: Additional patch, to cover slurmdbd's duplicate code

[Rémi Palancher]

Hi Karl,

Le 10/01/2017 à 22:31, Karl Kornel a écrit :

Hello!

I have a second patch, to make another similar change to my first patch.

I noticed that slurmdbd’s preinst script is doing the same user-creation.  
Although slurmdbd does not directly relate to slurm-client, both slurm-client 
and slurmdbd depend directly on the slurm-wlm-basic-plugins package.  So, my 
second patch does two things:

1) Move the preinst script from the slurm-client package to the 
slurm-wlm-basic-plugins package.
2) Remove the duplicate user-creation code from slurmdbd.


FWIW, we took the same approach in our EDF-specific packages:

https://github.com/edf-hpc/slurm-llnl

It works fine for us so far!

I just don't remember why we didn't propose the patch back into Debian 
official packages though :(


Best,
Rémi

Bug#850891: Additional patch, to cover slurmdbd's duplicate code

[Karl Kornel]

Hello!

I have a second patch, to make another similar change to my first patch.

I noticed that slurmdbd’s preinst script is doing the same user-creation.  
Although slurmdbd does not directly relate to slurm-client, both slurm-client 
and slurmdbd depend directly on the slurm-wlm-basic-plugins package.  So, my 
second patch does two things:

1) Move the preinst script from the slurm-client package to the 
slurm-wlm-basic-plugins package.
2) Remove the duplicate user-creation code from slurmdbd.

Again, please let me know if I am missing anything!

--
A. Karl Kornel | System Administrator
Research Computing | Stanford University
+1 (650) 736-9327



0002-Remove-duplicate-slurmdbd-user-creation-preinst.patch
Description: 0002-Remove-duplicate-slurmdbd-user-creation-preinst.patch