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

Single word title for Google Spreadsheet #6510

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Single word title for Google Spreadsheet #6510

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
81b9feb
Update google_spreadsheets.py
immortalcodes Oct 8, 2023
64f73ff
Update google_spreadsheets.py
immortalcodes Oct 16, 2023
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
32 changes: 17 additions & 15 deletions redash/query_runner/google_spreadsheets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,18 +100,20 @@ def __init__(self, worksheet_title):
def parse_query(query):
values = query.split("|")
key = values[0] # key of the spreadsheet
worksheet_num_or_title = 0 # A default value for when a number of inputs is invalid
worksheet_pointer = ' 1' # A default value for when a number of inputs is invalid
Copy link
Contributor

Choose a reason for hiding this comment

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

@immortalcodes hey, thanks for updating this PR. I think I might have misspoken though, or I wasn't clear in my communication - we want to separate parse_query into two versions of a similar function, where one uses a worksheet number and the other uses the title, and we want to call the correct function depending on what the user inputs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, we don't have to worry about things like a space prefix on a string signifying anything; we should keep the data as separate as possible from the code logic.

if len(values) == 2:
s = values[1].strip()
if len(s) > 0:
if re.match(r"^\"(.*?)\"$", s):
# A string quoted by " means a title of worksheet
worksheet_num_or_title = s[1:-1]
worksheet_pointer = s[1:-1]
else:
# if spreadsheet contains more than one worksheet - this is the number of it
worksheet_num_or_title = int(s)

return key, worksheet_num_or_title
if re.match(r"^\d+$", s.split()[0]):
# index is initialized with a space to differentiate from the title of worksheet
worksheet_pointer = ' '+s.split()[0]
else:
worksheet_pointer = s.split()[0]
return key, worksheet_pointer


def parse_worksheet(worksheet):
Expand All @@ -131,17 +133,17 @@ def parse_worksheet(worksheet):
return data


def parse_spreadsheet(spreadsheet, worksheet_num_or_title):
def parse_spreadsheet(spreadsheet, worksheet_pointer):
worksheet = None
if isinstance(worksheet_num_or_title, int):
worksheet = spreadsheet.get_worksheet_by_index(worksheet_num_or_title)
if worksheet_pointer[0] == ' ': # An index of worksheet
worksheet = spreadsheet.get_worksheet_by_index(int(worksheet_pointer))
if worksheet is None:
worksheet_count = len(spreadsheet.worksheets())
raise WorksheetNotFoundError(worksheet_num_or_title, worksheet_count)
elif isinstance(worksheet_num_or_title, str):
worksheet = spreadsheet.get_worksheet_by_title(worksheet_num_or_title)
raise WorksheetNotFoundError(int(worksheet_pointer), worksheet_count)
elif worksheet_pointer[0] == ' ': # A title of worksheet
worksheet = spreadsheet.get_worksheet_by_title(worksheet_pointer)
if worksheet is None:
raise WorksheetNotFoundByTitleError(worksheet_num_or_title)
raise WorksheetNotFoundByTitleError(worksheet_pointer)

worksheet_values = worksheet.get_all_values()

Expand Down Expand Up @@ -240,7 +242,7 @@ def test_connection(self):

def run_query(self, query, user):
logger.debug("Spreadsheet is about to execute query: %s", query)
key, worksheet_num_or_title = parse_query(query)
key, worksheet_pointer = parse_query(query)

try:
spreadsheet_service = self._get_spreadsheet_service()
Expand All @@ -250,7 +252,7 @@ def run_query(self, query, user):
else:
spreadsheet = spreadsheet_service.open_by_key(key)

data = parse_spreadsheet(SpreadsheetWrapper(spreadsheet), worksheet_num_or_title)
data = parse_spreadsheet(SpreadsheetWrapper(spreadsheet), worksheet_pointer)

return json_dumps(data), None
except gspread.SpreadsheetNotFound:
Expand Down
Loading