Skip to content

Commit

Permalink
Running black, and flake8
Browse files Browse the repository at this point in the history
  • Loading branch information
Boris committed Apr 24, 2024
1 parent bafa704 commit 215005f
Show file tree
Hide file tree
Showing 22 changed files with 461 additions and 120 deletions.
16 changes: 12 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,20 +1,28 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v3.2.0
rev: v4.5.0
hooks:
- id: trailing-whitespace
files: '\.py$'
- id: end-of-file-fixer
files: '\.py$'
- id: check-yaml
# Keep this unchanged for YAML files
- id: check-added-large-files
files: '\.py$'

- repo: https://github.com/psf/black
rev: 23.7.0
rev: 24.3.0
hooks:
- id: black
language_version: python3.12
files: '\.py$'

- repo: https://github.com/pycqa/flake8
rev: 6.1.0
rev: 7.0.0
hooks:
- id: flake8
args: [ --config, pyproject.toml ]
args:
- --config=server/pyproject.toml
- --ignore=E203 # Ignore E203 directly in the pre-commit configuration
files: '\.py$'
6 changes: 5 additions & 1 deletion import_specifications/clients/authclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,11 @@ def get_user(self, token):
err = ret.json()
except Exception:
ret.raise_for_status()
raise ValueError("Error connecting to auth service: {} {}\n{}".format(ret.status_code, ret.reason, err["error"]["message"]))
raise ValueError(
"Error connecting to auth service: {} {}\n{}".format(
ret.status_code, ret.reason, err["error"]["message"]
)
)

user = ret.json()["user_id"]
self._cache.add_valid_token(token, user)
Expand Down
16 changes: 12 additions & 4 deletions import_specifications/clients/baseclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ def _get_token(user_id, password, auth_svc):
# note that currently globus usernames, and therefore kbase usernames,
# cannot contain non-ascii characters. In python 2, quote doesn't handle
# unicode, so if this changes this client will need to change.
body = "user_id=" + _requests.utils.quote(user_id) + "&password=" + _requests.utils.quote(password) + "&fields=token"
body = (
"user_id=" + _requests.utils.quote(user_id) + "&password=" + _requests.utils.quote(password) + "&fields=token"
)
ret = _requests.post(auth_svc, data=body, allow_redirects=True)
status = ret.status_code
if status >= 200 and status <= 299:
Expand All @@ -51,7 +53,9 @@ def _get_token(user_id, password, auth_svc):
return tok["token"]


def _read_inifile(file=_os.environ.get("KB_DEPLOYMENT_CONFIG", _os.environ["HOME"] + "/.kbase_config")): # @ReservedAssignment
def _read_inifile(
file=_os.environ.get("KB_DEPLOYMENT_CONFIG", _os.environ["HOME"] + "/.kbase_config")
): # @ReservedAssignment
# Another bandaid to read in the ~/.kbase_config file if one is present
authdata = None
if _os.path.exists(file):
Expand Down Expand Up @@ -168,7 +172,9 @@ def _call(self, url, method, params, context=None):
arg_hash["context"] = context

body = _json.dumps(arg_hash, cls=_JSONObjectEncoder)
ret = _requests.post(url, data=body, headers=self._headers, timeout=self.timeout, verify=not self.trust_all_ssl_certificates)
ret = _requests.post(
url, data=body, headers=self._headers, timeout=self.timeout, verify=not self.trust_all_ssl_certificates
)
ret.encoding = "utf-8"
if ret.status_code == 500:
if ret.headers.get(_CT) == _AJ:
Expand All @@ -194,7 +200,9 @@ def _get_service_url(self, service_method, service_version):
if not self.lookup_url:
return self.url
service, _ = service_method.split(".")
service_status_ret = self._call(self.url, "ServiceWizard.get_service_status", [{"module_name": service, "version": service_version}])
service_status_ret = self._call(
self.url, "ServiceWizard.get_service_status", [{"module_name": service, "version": service_version}]
)
return service_status_ret["url"]

def _set_up_context(self, service_ver=None, context=None):
Expand Down
24 changes: 18 additions & 6 deletions import_specifications/clients/narrative_method_store_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,9 @@ def list_methods_full_info(self, params, context=None):
structure: parameter "pmid" of String, parameter "display_text" of
String, parameter "link" of type "url"
"""
return self._client.call_method("NarrativeMethodStore.list_methods_full_info", [params], self._service_ver, context)
return self._client.call_method(
"NarrativeMethodStore.list_methods_full_info", [params], self._service_ver, context
)

def list_methods_spec(self, params, context=None):
"""
Expand Down Expand Up @@ -548,7 +550,9 @@ def list_method_ids_and_names(self, params, context=None):
'release').) -> structure: parameter "tag" of String
:returns: instance of mapping from String to String
"""
return self._client.call_method("NarrativeMethodStore.list_method_ids_and_names", [params], self._service_ver, context)
return self._client.call_method(
"NarrativeMethodStore.list_method_ids_and_names", [params], self._service_ver, context
)

def list_apps(self, params, context=None):
"""
Expand Down Expand Up @@ -593,7 +597,9 @@ def list_apps_full_info(self, params, context=None):
parameter "screenshots" of list of type "ScreenShot" -> structure:
parameter "url" of type "url"
"""
return self._client.call_method("NarrativeMethodStore.list_apps_full_info", [params], self._service_ver, context)
return self._client.call_method(
"NarrativeMethodStore.list_apps_full_info", [params], self._service_ver, context
)

def list_apps_spec(self, params, context=None):
"""
Expand Down Expand Up @@ -685,7 +691,9 @@ def get_method_brief_info(self, params, context=None):
list of String, parameter "output_types" of list of String,
parameter "app_type" of String
"""
return self._client.call_method("NarrativeMethodStore.get_method_brief_info", [params], self._service_ver, context)
return self._client.call_method(
"NarrativeMethodStore.get_method_brief_info", [params], self._service_ver, context
)

def get_method_full_info(self, params, context=None):
"""
Expand Down Expand Up @@ -722,7 +730,9 @@ def get_method_full_info(self, params, context=None):
structure: parameter "pmid" of String, parameter "display_text" of
String, parameter "link" of type "url"
"""
return self._client.call_method("NarrativeMethodStore.get_method_full_info", [params], self._service_ver, context)
return self._client.call_method(
"NarrativeMethodStore.get_method_full_info", [params], self._service_ver, context
)

def get_method_spec(self, params, context=None):
"""
Expand Down Expand Up @@ -2452,7 +2462,9 @@ def load_widget_java_script(self, params, context=None):
parameter "tag" of String
:returns: instance of String
"""
return self._client.call_method("NarrativeMethodStore.load_widget_java_script", [params], self._service_ver, context)
return self._client.call_method(
"NarrativeMethodStore.load_widget_java_script", [params], self._service_ver, context
)

def register_repo(self, params, context=None):
"""
Expand Down
21 changes: 17 additions & 4 deletions import_specifications/generate_import_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,21 @@

def parse_args():
parser = argparse.ArgumentParser(description="Generate a bulk import template for an app")
parser.add_argument("app_id", help="The app ID to process, for example kb_uploadmethods/import_sra_as_reads_from_staging")
parser.add_argument("data_type", help="The datatype corresponding to the the app. This id is shared between the " + "staging service and the narrative, for example sra_reads")
parser.add_argument(
"app_id", help="The app ID to process, for example kb_uploadmethods/import_sra_as_reads_from_staging"
)
parser.add_argument(
"data_type",
help="The datatype corresponding to the the app. This id is shared between the "
+ "staging service and the narrative, for example sra_reads",
)
parser.add_argument("--tsv", action="store_true", help="Create a TSV file rather than a CSV file (the default)")
parser.add_argument("--env", choices=["prod", "appdev", "next", "ci"], default="prod", help="The KBase environment to query, default prod")
parser.add_argument(
"--env",
choices=["prod", "appdev", "next", "ci"],
default="prod",
help="The KBase environment to query, default prod",
)
parser.add_argument("--print-spec", action="store_true", help="Print the input specification for the app to stderr")
return parser.parse_args()

Expand Down Expand Up @@ -84,7 +95,9 @@ def main():
print(json.dumps(spec[0]["parameters"], indent=4), file=sys.stderr)
params = sort_params(spec[0]["parameters"])
sep = "\t" if args.tsv else ", "
print(f"Data type: {args.data_type}{_HEADER_SEP} " + f"Columns: {len(params)}{_HEADER_SEP} Version: {_FORMAT_VERSION}")
print(
f"Data type: {args.data_type}{_HEADER_SEP} " + f"Columns: {len(params)}{_HEADER_SEP} Version: {_FORMAT_VERSION}"
)
# we could theoretically use the parameter order to note for the users the type of each
# column - e.g. file input, output name, params, advanced params
# That's not in scope for now
Expand Down
4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
[tool.black]
line_length = 180
line_length = 120
multi_line_output = 3
extend-exclude = '''Pipfile.lock'''


[flake8]
max-line-length = 180
max-line-length = 120
exclude = '''Pipfile.lock'''
4 changes: 3 additions & 1 deletion scripts/prune_acls.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ def remove_acl(acl):
:param acl: ACL To Delete
:return: Logs success or failure of deleting this ACL to the log
"""
logging.info("{}:About to remove ACL {} for {} (> {} days)".format(current_time, acl["id"], acl["path"], THRESHOLD_DAYS))
logging.info(
"{}:About to remove ACL {} for {} (> {} days)".format(current_time, acl["id"], acl["path"], THRESHOLD_DAYS)
)
try:
globus_transfer_client.delete_endpoint_acl_rule(endpoint_id, acl["id"])
except TransferAPIError as error:
Expand Down
3 changes: 2 additions & 1 deletion scripts/refresh_token.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
client = globus_sdk.NativeAppAuthClient(CLIENT_ID)

##########################
# If we ever need to re-request the refreshable tokens, uncomment this section and use the globus account with admin permissions
# If we ever need to re-request the refreshable tokens,
# uncomment this section and use the globus account with admin permissions
# on the share to complete the web browser step.
##########################
"""
Expand Down
4 changes: 3 additions & 1 deletion staging_service/JGIMetadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ async def read_metadata_for(path: Path):
data = await json.read()
return decoder.decode(data)
else:
raise web.HTTPNotFound(text="could not find associated JGI metadata file for {path}".format(path=path.user_path))
raise web.HTTPNotFound(
text="could not find associated JGI metadata file for {path}".format(path=path.user_path)
)
42 changes: 31 additions & 11 deletions staging_service/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,16 @@ async def bulk_specification(request: web.Request) -> web.json_response:
p = Path.validate_path(username, f)
paths[PathPy(p.full_path)] = PathPy(p.user_path)
# list(dict) returns a list of the dict keys in insertion order (py3.7+)
res = parse_import_specifications(tuple(list(paths)), _file_type_resolver, lambda e: logging.error("Unexpected error while parsing import specs", exc_info=e))
res = parse_import_specifications(
tuple(list(paths)),
_file_type_resolver,
lambda e: logging.error("Unexpected error while parsing import specs", exc_info=e),
)
if res.results:
types = {dt: result.result for dt, result in res.results.items()}
files = {dt: {"file": str(paths[result.source.file]), "tab": result.source.tab} for dt, result in res.results.items()}
files = {
dt: {"file": str(paths[result.source.file]), "tab": result.source.tab} for dt, result in res.results.items()
}
return web.json_response({"types": types, "files": files})
errtypes = {e.error for e in res.errors}
errtext = json.dumps({"errors": format_import_spec_errors(res.errors, paths)})
Expand Down Expand Up @@ -161,9 +167,13 @@ async def write_bulk_specification(request: web.Request) -> web.json_response:
username = await authorize_request(request)
if request.content_type != _APP_JSON:
# There should be a way to get aiohttp to handle this but I can't find it
return _createJSONErrorResponse(f"Required content-type is {_APP_JSON}", error_class=web.HTTPUnsupportedMediaType)
return _createJSONErrorResponse(
f"Required content-type is {_APP_JSON}", error_class=web.HTTPUnsupportedMediaType
)
if not request.content_length:
return _createJSONErrorResponse("The content-length header is required and must be > 0", error_class=web.HTTPLengthRequired)
return _createJSONErrorResponse(
"The content-length header is required and must be > 0", error_class=web.HTTPLengthRequired
)
# No need to check the max content length; the server already does that. See tests
data = await request.json()
if type(data) != dict: # noqa E721
Expand Down Expand Up @@ -198,7 +208,9 @@ async def add_acl_concierge(request: web.Request):
aclm = AclManager()
result = aclm.add_acl_concierge(shared_directory=user_dir, concierge_path=concierge_path)
result["msg"] = f"Requesting Globus Perms for the following globus dir: {concierge_path}"
result["link"] = f"https://app.globus.org/file-manager?destination_id={aclm.endpoint_id}&destination_path={concierge_path}"
result[
"link"
] = f"https://app.globus.org/file-manager?destination_id={aclm.endpoint_id}&destination_path={concierge_path}"
return web.json_response(result)


Expand Down Expand Up @@ -400,13 +412,17 @@ async def upload_files_chunked(request: web.Request):

filename: str = user_file.filename
if filename.lstrip() != filename:
raise web.HTTPForbidden(text="cannot upload file with name beginning with space") # forbidden isn't really the right code, should be 400
raise web.HTTPForbidden(
text="cannot upload file with name beginning with space"
) # forbidden isn't really the right code, should be 400
if "," in filename:
raise web.HTTPForbidden(text="cannot upload file with ',' in name") # for consistency we use 403 again
# may want to make this configurable if we ever decide to add a hidden files toggle to
# the staging area UI
if filename.startswith("."):
raise web.HTTPForbidden(text="cannot upload file with name beginning with '.'") # for consistency we use 403 again
raise web.HTTPForbidden(
text="cannot upload file with name beginning with '.'"
) # for consistency we use 403 again

size = 0
destPath = os.path.join(destPath, filename)
Expand All @@ -421,7 +437,9 @@ async def upload_files_chunked(request: web.Request):
f.write(chunk)

if not os.path.exists(path.full_path):
error_msg = "We are sorry but upload was interrupted. Please try again.".format(path=path.full_path)
error_msg = "We are sorry but upload was interrupted. Please try again.".format( # noqa F522
path=path.full_path
)
raise web.HTTPNotFound(text=error_msg)

response = await some_metadata(
Expand All @@ -447,7 +465,7 @@ async def define_UPA(request: web.Request):
body = await request.post()
try:
UPA = body["UPA"]
except KeyError as wrong_key:
except KeyError:
raise web.HTTPBadRequest(text="must provide UPA field in body")
await add_upa(path, UPA)
return web.Response(text="succesfully updated UPA {UPA} for file {path}".format(UPA=UPA, path=path.user_path))
Expand Down Expand Up @@ -493,7 +511,7 @@ async def rename(request: web.Request):
body = await request.post()
try:
new_path = body["newPath"]
except KeyError as wrong_key:
except KeyError:
raise web.HTTPBadRequest(text="must provide newPath field in body")
new_path = Path.validate_path(username, new_path)
if os.path.exists(path.full_path):
Expand All @@ -505,7 +523,9 @@ async def rename(request: web.Request):
raise web.HTTPConflict(text="{new_path} allready exists".format(new_path=new_path.user_path))
else:
raise web.HTTPNotFound(text="{path} not found".format(path=path.user_path))
return web.Response(text="successfully moved {path} to {new_path}".format(path=path.user_path, new_path=new_path.user_path))
return web.Response(
text="successfully moved {path} to {new_path}".format(path=path.user_path, new_path=new_path.user_path)
)


@routes.patch("/decompress/{path:.+}")
Expand Down
14 changes: 12 additions & 2 deletions staging_service/app_error_formatter.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,18 @@
ErrorType.NO_FILES_PROVIDED: lambda msg, file1, tab1, file2, tab2: {
"type": "no_files_provided",
},
ErrorType.PARSE_FAIL: lambda msg, file1, tab1, file2, tab2: {"type": "cannot_parse_file", "message": msg, "file": file1, "tab": tab1},
ErrorType.INCORRECT_COLUMN_COUNT: lambda msg, file1, tab1, file2, tab2: {"type": "incorrect_column_count", "message": msg, "file": file1, "tab": tab1},
ErrorType.PARSE_FAIL: lambda msg, file1, tab1, file2, tab2: {
"type": "cannot_parse_file",
"message": msg,
"file": file1,
"tab": tab1,
},
ErrorType.INCORRECT_COLUMN_COUNT: lambda msg, file1, tab1, file2, tab2: {
"type": "incorrect_column_count",
"message": msg,
"file": file1,
"tab": tab1,
},
ErrorType.MULTIPLE_SPECIFICATIONS_FOR_DATA_TYPE: lambda msg, file1, tab1, file2, tab2: {
"type": "multiple_specifications_for_data_type",
"message": msg,
Expand Down
Loading

0 comments on commit 215005f

Please sign in to comment.