Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add new option to CLI to run job, poll for job status and output of job #262

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add new option to CLI to run job, poll for job status and output of job #262

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 67 additions & 4 deletions clients/tango-cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
import argparse
import sys
import os
import time
from datetime import datetime
import json
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove redundant import of json

The json module is already imported at line 10. Importing it again at line 17 is unnecessary and can be removed to avoid redundancy.

Apply this diff to remove the redundant import:

- import json
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import json
🧰 Tools
🪛 Ruff

17-17: Redefinition of unused json from line 10

Remove definition: json

(F811)

import uuid

sys.path.append("/usr/lib/python2.7/site-packages/")

Expand Down Expand Up @@ -69,6 +73,9 @@
parser.add_argument("--jobid", help="Job ID")

parser.add_argument("--runJob", help="Run a job from a specific directory")
parser.add_argument(
"--runJobPoll", help="Run a job from a specific directory, then poll"
)
parser.add_argument("--numJobs", type=int, default=1, help="Number of jobs to run")

parser.add_argument(
Expand Down Expand Up @@ -270,6 +277,8 @@ def tango_addJob():
% (args.server, args.port, args.key, args.courselab, json.dumps(requestObj))
)
print(response.text)
if args.runJob:
return response.text
Comment on lines +280 to +281
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure consistent return statements in tango_addJob

The function tango_addJob now returns response.text when args.runJob is set but does not return anything otherwise. This inconsistent return behavior may lead to issues if the calling code expects a return value.

Consider modifying the function to always return response.text for consistency:

 def tango_addJob():
     ...
     response = requests.post(...)
     print(...)
-    if args.runJob:
-        return response.text
+    return response.text
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if args.runJob:
return response.text
return response.text


except Exception as err:
print(
Expand Down Expand Up @@ -349,7 +358,10 @@ def tango_poll():
urllib.parse.quote(args.outputFile),
)
)
print(response.text)
if args.runJobPoll:
return response.text
else:
print(response.text)
Comment on lines +361 to +364
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure consistent return statements in tango_poll

The function tango_poll conditionally returns response.text when args.runJobPoll is set and prints the response otherwise. This inconsistent behavior could cause issues in functions that depend on its return value.

Modify the function to always return response.text and handle printing in the calling code:

 def tango_poll():
     ...
     response = requests.get(...)
     print(...)
-    if args.runJobPoll:
-        return response.text
-    else:
-        print(response.text)
+    return response.text
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if args.runJobPoll:
return response.text
else:
print(response.text)
return response.text


except Exception as err:
print(
Expand Down Expand Up @@ -407,7 +419,10 @@ def tango_jobs():
"Sent request to %s:%d/jobs/%s/%d/"
% (args.server, args.port, args.key, args.deadJobs)
)
print(response.text)
if args.runJobPoll:
return response.text
else:
print(response.text)
Comment on lines +422 to +425
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure consistent return statements in tango_jobs

Similar to tango_poll, the tango_jobs function conditionally returns response.text based on args.runJobPoll. This may lead to inconsistent behavior in the calling code.

Refactor the function to always return response.text:

 def tango_jobs():
     ...
     response = requests.get(...)
     print(...)
-    if args.runJobPoll:
-        return response.text
-    else:
-        print(response.text)
+    return response.text
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if args.runJobPoll:
return response.text
else:
print(response.text)
return response.text


except Exception as err:
print(
Expand Down Expand Up @@ -539,7 +554,6 @@ def tango_runJob():
if args.runJob is None:
print("Invalid usage: [runJob]")
sys.exit(0)

dir = args.runJob
infiles = [
file for file in os.listdir(dir) if os.path.isfile(os.path.join(dir, file))
Expand All @@ -549,6 +563,8 @@ def tango_runJob():

args.jobname += "-0"
args.outputFile += "-0"
# assuming we send a single job
job_id = 0
for i in range(1, args.numJobs + 1):
print(
"----------------------------------------- STARTING JOB "
Expand All @@ -565,10 +581,54 @@ def tango_runJob():
length = len(str(i - 1))
args.jobname = args.jobname[:-length] + str(i)
args.outputFile = args.outputFile[:-length] + str(i)
tango_addJob()
addJob_response = json.loads(tango_addJob())
if addJob_response["statusId"] == 0:
job_id = addJob_response["jobId"]
print(
"--------------------------------------------------------------------------------------------------\n"
)
return job_id


def tango_runJobPoll():
end_string = "Job exited"
timeout_string = "Job timed out"
args.courselab = uuid.uuid4()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Convert UUID to string when assigning to args.courselab

At line 596, args.courselab is assigned a UUID object. Since args.courselab is used in URL construction and string formatting, assigning a UUID object directly may cause issues.

Convert the UUID to a string when assigning to args.courselab:

- args.courselab = uuid.uuid4()
+ args.courselab = str(uuid.uuid4())
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
args.courselab = uuid.uuid4()
args.courselab = str(uuid.uuid4())

print(f"CREATE: RANDOM COURSELAB WITH ID: {args.courselab}")
args.runJob = args.runJobPoll
job_id = tango_runJob()
print("JOB ID", job_id)
while True:
print(
f"--------------------------------- POLL at {datetime.now()} ---------------------------------\n"
)
tango_info()
args.deadJobs = 0
jobs_response = tango_jobs()
jobs_json = json.loads(jobs_response)["jobs"]
for live_job in jobs_json:
if int(live_job["id"]) == int(job_id):
print(f"FOUND JOB {job_id} in live jobs: {live_job}")
args.deadJobs = 1
jobs_response = tango_jobs()
jobs_json = json.loads(jobs_response)["jobs"]
for dead_job in jobs_json:
if int(dead_job["id"]) == int(job_id):
print(f"FOUND JOB {job_id} in dead jobs: {dead_job}")
# print(jobs_json["jobs"])
print("----------- POLL FOR OUTPUT -----------\n")
response_text = tango_poll()
print(response_text)
print(
"--------------------------------------------------------------------------------------------------\n"
)
if end_string in response_text:
print("JOB EXITED SUCCESSFULLY")
return
elif timeout_string in response_text:
print("JOB TIMED OUT")
return
time.sleep(5)


def router():
Expand All @@ -594,6 +654,8 @@ def router():
tango_getPartialOutput()
elif args.build:
tango_build()
elif args.runJobPoll:
tango_runJobPoll()


#
Expand All @@ -612,6 +674,7 @@ def router():
and not args.runJob
and not args.getPartialOutput
and not args.build
and not args.runJobPoll
):
parser.print_help()
sys.exit(0)
Expand Down