Project

General

Profile

Feature #3777

Add DNs from a file for glidecondor_addDN

Added by Igor Sfiligoi over 6 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Igor Sfiligoi
Category:
-
Target version:
Start date:
04/26/2013
Due date:
% Done:

0%

Estimated time:
Stakeholders:
Duration:

Description

glidecondor_addDN currently only supports adding one DN at a time, and requires them to be passed as arguments.

This is particularly annoying when one is installing Condor from scratch, and wants to populate the mapfile
with all the needed DNs.

I thus propose to add the option to read the list from a file.

History

#1 Updated by Igor Sfiligoi over 6 years ago

  • Status changed from New to Assigned

Implemented the code changes in
branch_v2plus_igor_3777

Need to write documentation, but would appreciate feedback ASAP.

#2 Updated by Burt Holzman over 6 years ago

I didn't review the patch but just reviewed the script as of the last commit (commit:98644297) in the 3777 branch.

  1. We need the documentation on the formatting of the listfile obviously.
  2. The argument parsing is currently order-dependent, we should fix that: glidecondor_addDN -import dn.txt -q will not do what you want.
  3. I don't understand the (len(dnlist) != 1 or dnlist[0]['is_daemon_dn']) logic. Doesn't that imply different behavior based on the order of DNs
    in the import file?
  4. We seem to be catching and reraising IOErrors in some places. Some of that could be dropped if the catching statement doesn't add any
    useful information to the exception.
  5. Some of the code in parse_import_args could be cleaned up a bit, but that's more cosmetic; I'll take care of that before it gets merged.
  6. update_mapfile won't work on a brand new condor_mapfile (only the anon mappings), it goes looking for GSI ". Why not just prepend the new DN? (And if you want to keep this behavior, why use for i in range instead of for line in lines?

#3 Updated by Igor Sfiligoi over 6 years ago

All valid points, but let me just point out that most apply to the old/existing version as well.

I had tried to make a minimum number of changes, but you are right...
I could just do it right this time around.

I will have a new commit soon.

#4 Updated by Igor Sfiligoi over 6 years ago

I have cleaned up the code.
Please let me know if you still want changes.

PS: Documentation still missing.

#5 Updated by Igor Sfiligoi over 6 years ago

To help in understanding the code.
The file format of -import is
username type DN

where
  • type is either daemon or client
  • DN must not be quoted and can contain spaces.... any initial and trailing spaces are stripped off

Comments and empty lines are allowed.

#6 Updated by Burt Holzman over 6 years ago

Just skimmed the commit mails and am not trying to nitpick, but any reason you didn't switch over to optparse?

#7 Updated by Igor Sfiligoi over 6 years ago

Just a preference, I guess.

I don't find optparse really of any help.

#8 Updated by Igor Sfiligoi over 6 years ago

Documented the new syntax.

#9 Updated by Parag Mhashilkar over 6 years ago

  • Target version changed from v2_7_1 to v2_7_x

Talked to Igor and seems like after discussions between Igor and Burt, this will be part of v2.7.2

#10 Updated by Igor Sfiligoi over 6 years ago

  • Status changed from Assigned to Resolved

Merged into both v2plus and master.

#11 Updated by Parag Mhashilkar over 5 years ago

  • Status changed from Resolved to Closed
  • Target version changed from v2_7_x to v2_7_1


Also available in: Atom PDF