-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Improved handling of user query for higlass items #564
Changes from 5 commits
6f85d4e
19133ec
e420b06
745dc87
7c2d110
63ae933
f316761
4a61102
4920c97
037851a
f8e3a2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -354,12 +354,7 @@ def find_files_requiring_higlass_items(connection, check_name, action_name, sear | |
check.queries = [] | ||
check.action = action_name | ||
|
||
# If no search query was provided, fail | ||
if not search_queries: | ||
check.summary = check.description = "No search query provided, nothing to update." | ||
check.status = 'PASS' | ||
check.allow_action = False | ||
return check | ||
check, search_queries = verify_queries(check, search_queries, False) | ||
|
||
# Add the fields we want to return. | ||
fields_to_include = '&field=' + '&field='.join(( | ||
|
@@ -827,12 +822,7 @@ def find_expsets_processedfiles_requiring_higlass_items(connection, check_name, | |
"static_content", | ||
]) | ||
|
||
# If no search query was provided, fail | ||
if not search_queries: | ||
check.summary = check.description = "No search query provided, nothing to update." | ||
check.status = 'PASS' | ||
check.allow_action = False | ||
return check | ||
check, search_queries = verify_queries(check, search_queries, False) | ||
|
||
expsets_by_accession = {} | ||
# Use all of the search queries to make a list of the ExpSets we will work on. | ||
|
@@ -1177,12 +1167,7 @@ def find_expsets_otherprocessedfiles_requiring_higlass_items(connection, check_n | |
check.queries = [] | ||
check.action = action_name | ||
|
||
# If no search query was provided and find_opfs_missing_higlass is False, pass with no results | ||
if not (search_queries or find_opfs_missing_higlass): | ||
check.summary = check.description = "No search query provided, nothing to update." | ||
check.status = 'PASS' | ||
check.allow_action = False | ||
return check | ||
check, search_queries = verify_queries(check, search_queries, find_opfs_missing_higlass) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can simplify the verify_queries function and get rid of the ignore_queries parameter if you move this call into an else after the if at 1172 where if the find_opfs_missing_higlass is passed in it uses the query on the next couple of lines else you can call the verify_queries function. |
||
|
||
if find_opfs_missing_higlass: | ||
search_queries = [ | ||
|
@@ -2287,3 +2272,31 @@ def convert_es_timestamp_to_datetime(raw): | |
"%Y-%m-%dT%H:%M:%S" | ||
) | ||
return converted_date | ||
|
||
def verify_queries(check, search_queries, ignore_queries): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above - I think you can get rid of the ignore_queries parameter |
||
""" | ||
Helper to check that a search query if properly formatted and reformat if necessary | ||
|
||
Args: | ||
check(CheckResult): Result of check, to be passed from check. | ||
search_queries(list or string): A list of search queries. All Files found in at least one of the queries will be modified. | ||
ignore_queries(Boolean): If True, ignore search queries (used for other processed files default query) | ||
|
||
Returns: | ||
Check results object | ||
Formatted search_queries (list) | ||
""" | ||
# If no search query was provided (and ignore_queries is False), pass with no results | ||
if not (search_queries or ignore_queries): | ||
check.summary = check.description = "No search query provided, nothing to update." | ||
check.status = 'PASS' | ||
check.allow_action = False | ||
|
||
if isinstance(search_queries, str): | ||
# for case where (possibly multiple) query is passed in via kwargs | ||
queries = search_queries.split(',') | ||
search_queries = [q.strip() for q in queries] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it might also be useful to check if the passed query(s) begin with an & and if not add it. |
||
check.brief_output = { | ||
"corrected_query": "The query was not formatted as a list, please double check results" | ||
} | ||
return check, search_queries |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
[tool.poetry] | ||
name = "foursight" | ||
version = "4.4.2" | ||
version = "4.4.3" | ||
description = "Serverless Chalice Application for Monitoring" | ||
authors = ["4DN-DCIC Team <[email protected]>"] | ||
license = "MIT" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe you need to return the check from this function - you are passing a reference to an object which can be modified in the function and that change will be reflected without needing to return it.