Project

General

Profile

Bug #24213

Bug #21031: jobsub_submit allows more than one --group argument

Review request [commit:c56de182a5c7a2f3cbdb6d6de5d254b1fddeb434: Added iteritems() to vars call]

Added by Shreyas Bhat 5 months ago. Updated 4 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Target version:
-
Start date:
03/24/2020
Due date:
% Done:

0%

Estimated time:
First Occurred:
Occurs In:
Stakeholders:
Duration:

Description

I think I've fixed all the underlying issues. Please review this branch/commit and let me know if everything looks good.

You generally run a new release through the test suite, right?

Thanks!
Shreyas

History

#1 Updated by Dennis Box 4 months ago

  • Assignee changed from Dennis Box to Shreyas Bhat
  • Status changed from New to Accepted

The code changes look reasonable, and I tested them a bit. I confirm that they seem to work for --group or -G for jobsub_submit and jobsub_submit_dag. I notice they do not work for other commands, I don't know that this is a high priority.
Please merge this change into master. We should discuss adding some tests to the test suite tomorrow after the jobsub meeting.

#2 Updated by Shreyas Bhat 4 months ago

That's a good point. I purposefully only changed jobsub_submit and jobsub_submit_dag, but perhaps this should be a changed for all commands, since they all require the -G flag. Would you like me to do that this release or next one?

On that note, perhaps we should consider making a parser for the jobsub client that all the commands can inherit and then build on top of. What are your thoughts on that? I could slate that for next release, but it shouldn't be difficult once we know the flags that are common among all the commands.

#3 Updated by Shreyas Bhat 4 months ago

Talked to Dennis offline - we'll put in a new ticket for this and put it in the next release.

#4 Updated by Shreyas Bhat 4 months ago

  • Status changed from Accepted to Resolved

Merged and pushed to master.

#5 Updated by Shreyas Bhat 4 months ago

  • Status changed from Resolved to Closed

#6 Updated by Dennis Box 4 months ago

  • Target version deleted (v1.3.2)


Also available in: Atom PDF