Project

General

Profile

Bug #2634

Support #2139: Check support for platform SL6

Migrate popen calls to subprocess for SL6 compatibility

Added by Burt Holzman over 8 years ago. Updated over 7 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Parag Mhashilkar
Category:
-
Target version:
Start date:
10/04/2012
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
First Occurred:
Occurs In:
Stakeholders:
Duration:

Subtasks

Bug #2984: Problematic condorExe behaviorClosedBurt Holzman

Bug #3131: Master: Migrate popen calls to subprocess for SL6 compatibility ClosedParag Mhashilkar

History

#1 Updated by Anthony Tiradani over 8 years ago

Pushed branch_v2_5_5plus_2634 - This code has not been tested so may be (probably is) broken.

#2 Updated by Burt Holzman about 8 years ago

  • Status changed from New to Feedback
  • Assignee changed from Anthony Tiradani to Parag Mhashilkar

Assigning to Parag for review, since this has big potential performance gains for the factory.

#3 Updated by John Weigand about 8 years ago

Started tests yesterday 11/6 and had it run through the night with user jobs being submitted continually.
Appeared to have no problems. The only change needed was to:
- creation/lib/cvWCreate.py hash: fd949ca755b7b3d4d343017f8db5f099c078ebe2

RRDs, best i can tell, look fine.

John Weigand

#4 Updated by Parag Mhashilkar about 8 years ago

  • Assignee changed from Parag Mhashilkar to Douglas Strain

I made quite a bit of changes to get it working. John has done the testing and so far hasn't found any runtime issue. Doug can you please review the code? Its in branch_v2plus-2634 and has v2.6.2 merged into it. So diffing it wrt to v2_6_2 tag should give you the changes made to use subprocess.

#5 Updated by Parag Mhashilkar about 8 years ago

  • Target version changed from v2_7_x to 293

#6 Updated by Douglas Strain about 8 years ago

  • Assignee changed from Douglas Strain to Parag Mhashilkar

Ok, I have reviewed the code and have also deployed a one-node install using this branch. No problems with the install or glidein submission. Only minor issues with the code review. No show-stoppers, so feel free to merge once you look at the below list and do any of them you want.

Here are my comments from the code review:
1) Both ini installer and Q&A installer use os.popen and not subprocess
2) which function in glideFactoryLib is useful. I think this should be pulled out into lib/subprocessSupport or exeSupport or something.
3) which function should also be documented more. The behavior for which('/tmp/python') for instance is to return None. That's the same as the which command, but could be clarified.
4) rrdtool_exe should use which
5) condorExe could use which
6) in iexe_cmd there is a comment "This should eventually do the right thing". I am not sure what it means, but it scares me.

#7 Updated by Burt Holzman about 8 years ago

These are good comments, Doug. I don't see any showstoppers for merging, so I think we should merge 2634 in do branch_v2plus.

I'll open new tickets to address 1, 2-5, and 6.

#8 Updated by Parag Mhashilkar about 8 years ago

Merged to v2plus. I will open a ticket for similar change to master.

#9 Updated by Parag Mhashilkar about 8 years ago

  • Status changed from Feedback to Resolved

#10 Updated by Parag Mhashilkar over 7 years ago

  • Target version changed from 293 to v2_7

#11 Updated by Parag Mhashilkar over 7 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF