Skip to content

Commit

Permalink
DEVEX-2424 Replace pipes for python 3.13 (#1410)
Browse files Browse the repository at this point in the history
  • Loading branch information
branchvincent authored Oct 22, 2024
1 parent 39b0847 commit e788e91
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 47 deletions.
4 changes: 2 additions & 2 deletions src/python/dxpy/cli/exec_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

# TODO: refactor all dx run helper functions here

import os, sys, json, collections, pipes
import os, sys, json, collections, shlex
from ..bindings.dxworkflow import DXWorkflow

import dxpy
Expand Down Expand Up @@ -327,7 +327,7 @@ def format_data_object_reference(item):
# TODO: in interactive prompts the quotes here may be a bit
# misleading. Perhaps it should be a separate mode to print
# "interactive-ready" suggestions.
return fill(header + ' ' + ', '.join([pipes.quote(str(item)) for item in items]),
return fill(header + ' ' + ', '.join([shlex.quote(str(item)) for item in items]),
initial_indent=initial_indent,
subsequent_indent=subsequent_indent)

Expand Down
4 changes: 2 additions & 2 deletions src/python/dxpy/utils/exec_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import os, sys, json, re, collections, logging, argparse, string, itertools, subprocess, tempfile
from functools import wraps
from collections import namedtuple
import pipes
import shlex

import dxpy
from ..compat import USING_PYTHON2, open, Mapping
Expand Down Expand Up @@ -435,7 +435,7 @@ def _install_dep_bundle(self, bundle):
dxpy.download_dxfile(bundle["id"], bundle["name"], project=dxpy.WORKSPACE_ID)
except dxpy.exceptions.ResourceNotFound:
dxpy.download_dxfile(bundle["id"], bundle["name"])
self.run("dx-unpack {}".format(pipes.quote(bundle["name"])))
self.run("dx-unpack {}".format(shlex.quote(bundle["name"])))
else:
self.log('Skipping bundled dependency "{name}" because it does not refer to a file'.format(**bundle))

Expand Down
8 changes: 2 additions & 6 deletions src/python/dxpy/utils/file_load_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
from __future__ import print_function, unicode_literals, division, absolute_import

import json
import pipes
import shlex
import os
import fnmatch
import sys
Expand Down Expand Up @@ -401,10 +401,6 @@ def factory():
return file_key_descs, rest_hash


#
# Note: pipes.quote() to be replaced with shlex.quote() in Python 3
# (see http://docs.python.org/2/library/pipes.html#pipes.quote)
#
def gen_bash_vars(job_input_file, job_homedir=None, check_name_collision=True):
"""
:param job_input_file: path to a JSON file describing the job inputs
Expand All @@ -427,7 +423,7 @@ def string_of_elem(elem):
result = json.dumps(dxpy.dxlink(elem))
else:
result = json.dumps(elem)
return pipes.quote(result)
return shlex.quote(result)

def string_of_value(val):
if isinstance(val, list):
Expand Down
8 changes: 4 additions & 4 deletions src/python/dxpy/utils/local_exec_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

from __future__ import print_function, unicode_literals, division, absolute_import

import os, sys, json, subprocess, pipes
import os, sys, json, subprocess, shlex
import collections, datetime

import dxpy
Expand Down Expand Up @@ -351,9 +351,9 @@ def run_one_entry_point(job_id, function, input_hash, run_spec, depends_on, name
if [[ $(type -t {function}) == "function" ]];
then {function};
else echo "$0: Global scope execution complete. Not invoking entry point function {function} because it was not found" 1>&2;
fi'''.format(homedir=pipes.quote(job_homedir),
env_path=pipes.quote(os.path.join(job_env['HOME'], 'environment')),
code_path=pipes.quote(environ['DX_TEST_CODE_PATH']),
fi'''.format(homedir=shlex.quote(job_homedir),
env_path=shlex.quote(os.path.join(job_env['HOME'], 'environment')),
code_path=shlex.quote(environ['DX_TEST_CODE_PATH']),
function=function)
invocation_args = ['bash', '-c', '-e'] + (['-x'] if environ.get('DX_TEST_X_FLAG') else []) + [script]
elif run_spec['interpreter'] == 'python2.7':
Expand Down
1 change: 0 additions & 1 deletion src/python/test/test_dx_app_wizard.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import os, sys, unittest, json, tempfile, subprocess
import pexpect
import pipes

from dxpy_testutil import DXTestCase, check_output
import dxpy_testutil as testutil
Expand Down
20 changes: 10 additions & 10 deletions src/python/test/test_dx_bash_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import dxpy_testutil as testutil
import json
import os
import pipes
import shlex
import pytest
import shutil
import tempfile
Expand All @@ -42,7 +42,7 @@
def run(command, **kwargs):
try:
if isinstance(command, list) or isinstance(command, tuple):
print("$ %s" % " ".join(pipes.quote(f) for f in command))
print("$ %s" % " ".join(shlex.quote(f) for f in command))
output = check_output(command, **kwargs)
else:
print("$ %s" % (command,))
Expand Down Expand Up @@ -919,30 +919,30 @@ def test_job_arguments(self):
),
# instance type: mapping
("--instance-type " +
pipes.quote(json.dumps({"main": "mem2_hdd2_x2" , "other_function": "mem2_hdd2_x1" })),
shlex.quote(json.dumps({"main": "mem2_hdd2_x2" , "other_function": "mem2_hdd2_x1" })),
{"systemRequirements": {"main": { "instanceType": "mem2_hdd2_x2" },
"other_function": { "instanceType": "mem2_hdd2_x1" }}}),
("--instance-type-by-executable " +
pipes.quote(json.dumps({"my_applet": {"main": "mem2_hdd2_x2",
shlex.quote(json.dumps({"my_applet": {"main": "mem2_hdd2_x2",
"other_function": "mem3_ssd2_fpga1_x8"}})),
{"systemRequirementsByExecutable": {"my_applet": {"main": {"instanceType": "mem2_hdd2_x2"},
"other_function": {"instanceType": "mem3_ssd2_fpga1_x8"}}}}),
("--instance-type-by-executable " +
pipes.quote(json.dumps({"my_applet": {"main": "mem1_ssd1_v2_x2",
shlex.quote(json.dumps({"my_applet": {"main": "mem1_ssd1_v2_x2",
"other_function": "mem3_ssd2_fpga1_x8"}})) +
" --extra-args " +
pipes.quote(json.dumps({"systemRequirementsByExecutable": {"my_applet": {"main": {"instanceType": "mem2_hdd2_x2", "clusterSpec": {"initialInstanceCount": 3}},
shlex.quote(json.dumps({"systemRequirementsByExecutable": {"my_applet": {"main": {"instanceType": "mem2_hdd2_x2", "clusterSpec": {"initialInstanceCount": 3}},
"other_function": {"fpgaDriver": "edico-1.4.5"}}}})),
{"systemRequirementsByExecutable": {"my_applet":{"main": { "instanceType": "mem2_hdd2_x2", "clusterSpec":{"initialInstanceCount": 3}},
"other_function": { "instanceType": "mem3_ssd2_fpga1_x8", "fpgaDriver": "edico-1.4.5"} }}}),
# nvidia driver
("--instance-type-by-executable " +
pipes.quote(json.dumps({
shlex.quote(json.dumps({
"my_applet": {
"main": "mem1_ssd1_v2_x2",
"other_function": "mem2_ssd1_gpu_x16"}})) +
" --extra-args " +
pipes.quote(json.dumps({
shlex.quote(json.dumps({
"systemRequirementsByExecutable": {
"my_applet": {
"main": {"instanceType": "mem2_hdd2_x2"},
Expand All @@ -963,14 +963,14 @@ def test_job_arguments(self):
self.assertNewJobInputHash(cmd_snippet, arguments_hash)

def test_extra_arguments(self):
cmd_snippet = "--extra-args " + pipes.quote(
cmd_snippet = "--extra-args " + shlex.quote(
json.dumps({"details": {"d1": "detail1", "d2": 1234}, "foo": "foo_value"})
)
arguments_hash = {"details": {"d1": "detail1", "d2": 1234}, "foo": "foo_value"}
self.assertNewJobInputHash(cmd_snippet, arguments_hash)

# override previously specified args
cmd_snippet = "--name JobName --extra-args " + pipes.quote(
cmd_snippet = "--name JobName --extra-args " + shlex.quote(
json.dumps({"name": "FinalName"})
)
arguments_hash = {"name": "FinalName"}
Expand Down
44 changes: 22 additions & 22 deletions src/python/test/test_dxclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import os, sys, unittest, json, tempfile, subprocess, shutil, re, base64, random, time
import filecmp
import pipes
import shlex
import stat
import hashlib
import collections
Expand Down Expand Up @@ -380,14 +380,14 @@ def test_dx_set_details_with_file(self):

# Test -f with valid JSON file.
record_id = run("dx new record Ψ2 --brief").strip()
run("dx set_details Ψ2 -f " + pipes.quote(tmp_file.name))
run("dx set_details Ψ2 -f " + shlex.quote(tmp_file.name))
dxrecord = dxpy.DXRecord(record_id)
details = dxrecord.get_details()
self.assertEqual({"foo": "bar"}, details, msg="dx set_details -f with valid JSON input file failed.")

# Test --details-file with valid JSON file.
record_id = run("dx new record Ψ3 --brief").strip()
run("dx set_details Ψ3 --details-file " + pipes.quote(tmp_file.name))
run("dx set_details Ψ3 --details-file " + shlex.quote(tmp_file.name))
dxrecord = dxpy.DXRecord(record_id)
details = dxrecord.get_details()
self.assertEqual({"foo": "bar"}, details,
Expand All @@ -400,16 +400,16 @@ def test_dx_set_details_with_file(self):
# Test above with invalid JSON file.
record_id = run("dx new record Ψ4 --brief").strip()
with self.assertSubprocessFailure(stderr_regexp="JSON", exit_code=3):
run("dx set_details Ψ4 -f " + pipes.quote(tmp_invalid_file.name))
run("dx set_details Ψ4 -f " + shlex.quote(tmp_invalid_file.name))

# Test command with (-f or --details-file) and CL JSON.
with self.assertSubprocessFailure(stderr_regexp="Error: Cannot provide both -f/--details-file and details",
exit_code=3):
run("dx set_details Ψ4 '{ \"foo\":\"bar\" }' -f " + pipes.quote(tmp_file.name))
run("dx set_details Ψ4 '{ \"foo\":\"bar\" }' -f " + shlex.quote(tmp_file.name))

# Test piping JSON from STDIN.
record_id = run("dx new record Ψ5 --brief").strip()
run("cat " + pipes.quote(tmp_file.name) + " | dx set_details Ψ5 -f -")
run("cat " + shlex.quote(tmp_file.name) + " | dx set_details Ψ5 -f -")
dxrecord = dxpy.DXRecord(record_id)
details = dxrecord.get_details()
self.assertEqual({"foo": "bar"}, details, msg="dx set_details -f - with valid JSON input failed.")
Expand Down Expand Up @@ -5504,11 +5504,11 @@ def test_dx_find_data_by_region(self):
def test_dx_find_projects(self):
unique_project_name = 'dx find projects test ' + str(time.time())
with temporary_project(unique_project_name) as unique_project:
self.assertEqual(run("dx find projects --name " + pipes.quote(unique_project_name)),
self.assertEqual(run("dx find projects --name " + shlex.quote(unique_project_name)),
unique_project.get_id() + ' : ' + unique_project_name + ' (ADMINISTER)\n')
self.assertEqual(run("dx find projects --brief --name " + pipes.quote(unique_project_name)),
self.assertEqual(run("dx find projects --brief --name " + shlex.quote(unique_project_name)),
unique_project.get_id() + '\n')
json_output = json.loads(run("dx find projects --json --name " + pipes.quote(unique_project_name)))
json_output = json.loads(run("dx find projects --json --name " + shlex.quote(unique_project_name)))
self.assertEqual(len(json_output), 1)
self.assertEqual(json_output[0]['id'], unique_project.get_id())

Expand All @@ -5529,23 +5529,23 @@ def test_dx_find_projects_by_created(self):
created_project_name = 'dx find projects test ' + str(time.time())
with temporary_project(created_project_name) as unique_project:
self.assertEqual(run("dx find projects --created-after=-1d --brief --name " +
pipes.quote(created_project_name)), unique_project.get_id() + '\n')
shlex.quote(created_project_name)), unique_project.get_id() + '\n')
self.assertEqual(run("dx find projects --created-before=" + str(int(time.time() + 1000) * 1000) +
" --brief --name " + pipes.quote(created_project_name)),
" --brief --name " + shlex.quote(created_project_name)),
unique_project.get_id() + '\n')
self.assertEqual(run("dx find projects --created-after=-1d --created-before=" +
str(int(time.time() + 1000) * 1000) + " --brief --name " +
pipes.quote(created_project_name)), unique_project.get_id() + '\n')
shlex.quote(created_project_name)), unique_project.get_id() + '\n')
self.assertEqual(run("dx find projects --created-after=" + str(int(time.time() + 1000) * 1000) + " --name "
+ pipes.quote(created_project_name)), "")
+ shlex.quote(created_project_name)), "")

def test_dx_find_projects_by_region(self):
awseast = "aws:us-east-1"
azurewest = "azure:westus"
created_project_name = 'dx find projects test ' + str(time.time())
with temporary_project(created_project_name, region=awseast) as unique_project:
self.assertEqual(run("dx find projects --region {} --brief --name {}".format(
awseast, pipes.quote(created_project_name))),
awseast, shlex.quote(created_project_name))),
unique_project.get_id() + '\n')
self.assertIn(unique_project.get_id(),
run("dx find projects --region {} --brief".format(awseast)))
Expand Down Expand Up @@ -5635,10 +5635,10 @@ def test_dx_find_projects_by_property(self):
def test_dx_find_projects_phi(self):
projectName = "tempProject+{t}".format(t=time.time())
with temporary_project(name=projectName) as project_1:
res = run('dx find projects --phi true --brief --name ' + pipes.quote(projectName))
res = run('dx find projects --phi true --brief --name ' + shlex.quote(projectName))
self.assertTrue(len(res) == 0, "Expected no PHI projects to be found")

res = run('dx find projects --phi false --brief --name ' + pipes.quote(projectName)).strip().split('\n')
res = run('dx find projects --phi false --brief --name ' + shlex.quote(projectName)).strip().split('\n')
self.assertTrue(len(res) == 1, "Expected to find one project")
self.assertTrue(res[0] == project_1.get_id())

Expand Down Expand Up @@ -6287,10 +6287,10 @@ def test_dx_find_org_projects_phi(self):
project1_id = project_1.get_id()
dxpy.api.project_update(project1_id, {"billTo": self.org_id})

res = run('dx find org projects org-piratelabs --phi true --brief --name ' + pipes.quote(projectName))
res = run('dx find org projects org-piratelabs --phi true --brief --name ' + shlex.quote(projectName))
self.assertTrue(len(res) == 0, "Expected no PHI projects to be found")

res = run('dx find org projects org-piratelabs --phi false --brief --name ' + pipes.quote(projectName)).strip().split("\n")
res = run('dx find org projects org-piratelabs --phi false --brief --name ' + shlex.quote(projectName)).strip().split("\n")

self.assertTrue(len(res) == 1, "Expected to find one project")
self.assertEqual(res[0], project1_id)
Expand Down Expand Up @@ -7431,7 +7431,7 @@ def test_update_strings(self):

#Update items one by one.
for item in update_items:
run(self.cmd.format(pid=self.project, item=item, n=pipes.quote(update_items[item])))
run(self.cmd.format(pid=self.project, item=item, n=shlex.quote(update_items[item])))
describe_input = {}
describe_input[item] = 'true'
self.assertEqual(self.project_describe(describe_input)[item],
Expand All @@ -7447,14 +7447,14 @@ def test_update_multiple_items(self):
'protected': 'false'}

update_project_output = check_output(["dx", "update", "project", self.project, "--name",
pipes.quote(update_items['name']), "--summary", update_items['summary'], "--description",
shlex.quote(update_items['name']), "--summary", update_items['summary'], "--description",
update_items['description'], "--protected", update_items['protected']])
update_project_json = json.loads(update_project_output);
self.assertTrue("id" in update_project_json)
self.assertEqual(self.project, update_project_json["id"])

update_project_output = check_output(["dx", "update", "project", self.project, "--name",
pipes.quote(update_items['name']), "--summary", update_items['summary'], "--description",
shlex.quote(update_items['name']), "--summary", update_items['summary'], "--description",
update_items['description'], "--protected", update_items['protected'], "--brief"])
self.assertEqual(self.project, update_project_output.rstrip("\n"))

Expand All @@ -7479,7 +7479,7 @@ def test_update_project_by_name(self):
project_name = self.project_describe(describe_input)['name']
new_name = 'Another Project Name' + str(time.time())

run(self.cmd.format(pid=project_name, item='name', n=pipes.quote(new_name)))
run(self.cmd.format(pid=project_name, item='name', n=shlex.quote(new_name)))
result = self.project_describe(describe_input)
self.assertEqual(result['name'], new_name)

Expand Down

0 comments on commit e788e91

Please sign in to comment.