Skip to content
3 changes: 2 additions & 1 deletion dpdispatcher/contexts/openapi_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ def __init__(
self.init_remote_root = remote_root
self.temp_local_root = os.path.abspath(local_root)
self.remote_profile = remote_profile
os.makedirs(DP_CLOUD_SERVER_HOME_DIR, exist_ok=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove duplicate directory creation.

The directory creation on line 75 duplicates the identical call on line 105 within the same __init__ method.

Apply this diff to remove the duplicate:

-        os.makedirs(DP_CLOUD_SERVER_HOME_DIR, exist_ok=True)
         access_key = (

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In dpdispatcher/contexts/openapi_context.py around line 75, there's a duplicate
call to os.makedirs(DP_CLOUD_SERVER_HOME_DIR, exist_ok=True) that is repeated
later at line 105 in the same __init__ method; remove the earlier call at line
75 so the directory is only created once (keep the existing call at line 105),
and run tests/lint to ensure no other side-effects.

access_key = (
remote_profile.get("access_key", None)
or os.getenv("BOHRIUM_ACCESS_KEY", None)
Expand Down Expand Up @@ -170,7 +171,7 @@ def upload_job(self, job, common_files=None):
object_key = os.path.join(data["storePath"], zip_filename) # type: ignore
job.upload_path = object_key
job.job_id = data["jobId"] # type: ignore
job.jgid = data["jobGroupId"] # type: ignore
job.jgid = data.get("jobGroupId", "") # type: ignore
self.storage.upload_From_file_multi_part(
object_key=object_key, file_path=upload_zip, token=token
)
Expand Down
15 changes: 9 additions & 6 deletions dpdispatcher/machines/openapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,13 @@ def do_submit(self, job):
),
"out_files": self._gen_backward_files_list(job),
"platform": self.remote_profile.get("platform", "ali"),
"image_address": self.remote_profile.get("image_address", ""),
"image_name": self.remote_profile.get("image_address", ""),
}
if job.job_state == JobStatus.unsubmitted:
openapi_params["job_id"] = job.job_id
if "real_user_id" in self.remote_profile:
openapi_params["real_user_id"] = self.remote_profile["real_user_id"]
if "session_id" in self.remote_profile:
openapi_params["session_id"] = self.remote_profile["session_id"]
openapi_params["job_id"] = job.job_id
data = self.job.insert(**openapi_params)

job.job_id = data.get("jobId", 0) # type: ignore
Expand Down Expand Up @@ -182,8 +185,8 @@ def check_status(self, job):
self.ignore_exit_code,
)
if job_state == JobStatus.finished:
job_log = self.job.log(job_id)
if self.remote_profile.get("output_log"):
job_log = self.job.log(job_id)
print(job_log, end="")
self._download_job(job)
elif self.remote_profile.get("output_log") and job_state == JobStatus.running:
Expand All @@ -193,7 +196,7 @@ def check_status(self, job):

def _download_job(self, job):
data = self.job.detail(job.job_id)
job_url = data["jobFiles"]["outFiles"][0]["url"] # type: ignore
job_url = data["resultUrl"] # type: ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Use .get() for safer dictionary access.

Direct indexing data["resultUrl"] will raise KeyError if the key is missing. For consistency with the changes in openapi_context.py (lines 168, 174), use .get() with a default value.

Apply this diff:

-        job_url = data["resultUrl"]  # type: ignore
+        job_url = data.get("resultUrl", "")  # type: ignore
🤖 Prompt for AI Agents
In dpdispatcher/machines/openapi.py around line 199, the code uses direct
indexing data["resultUrl"] which can raise KeyError if the key is missing;
change it to use data.get("resultUrl", None) (or "" if an empty string is
preferable) to match the safer pattern used in openapi_context.py (lines 168,
174), and preserve any existing type comments (e.g., keep "# type: ignore" if
needed).

if not job_url:
return
job_hash = job.job_hash
Expand Down Expand Up @@ -243,7 +246,7 @@ def map_dp_job_state(status, exit_code, ignore_exit_code=True):
if status not in map_dict:
dlog.error(f"unknown job status {status}")
return JobStatus.unknown
if status == -1 and exit_code != 0 and ignore_exit_code:
if status == -1 and ignore_exit_code:
return JobStatus.finished
return map_dict[status]

Expand Down
Loading