-
Notifications
You must be signed in to change notification settings - Fork 0
YC Demo #34
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
base: main
Are you sure you want to change the base?
YC Demo #34
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| from flask import Flask, request, jsonify | ||
| from slack_sdk import WebClient | ||
| from google.oauth2.service_account import Credentials | ||
| from googleapiclient.discovery import build | ||
| import requests, json | ||
| import logging | ||
| import datetime | ||
|
|
||
|
|
||
| #logging | ||
| logging.basicConfig(level=logging.INFO) | ||
|
|
||
| #service | ||
| #start service here | ||
|
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. Comments — The commented-out code '#start service here' is unnecessary and does not provide any additional information or context. Suggestion: Remove the comment '#start service here'. |
||
| SCOPES = ['INSERT SCOPES'] | ||
| creds = Credentials.from_service_account_file('slack_key.json', scopes=SCOPES) #IMPORT SLACK KEY | ||
| service = build('sheets', 'v4', credentials=creds) | ||
| spreadsheet_id = 'SPREADSHEET KEY' #should set as environment variable | ||
| slack_token = 'SLACK KEY' | ||
| client = WebClient(token=slack_token) | ||
|
|
||
| #app | ||
| app = Flask(__name__) | ||
|
|
||
| #populate variables | ||
|
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. Naming — The constant value '3600' is used for calculating time which is not immediately obvious as 'one hour' to someone unfamiliar with the context. Suggestion: Define a constant with a descriptive name, such as SECONDS_IN_AN_HOUR, and use that instead of the magic number '3600'. |
||
| name = "" | ||
|
Comment on lines
+22
to
+26
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. Naming — Variable names are defined as global strings but are populated later in different functions, making the code hard to follow and understand. It's unclear when and how these are supposed to be used just from reading the initialization. Suggestion: Avoid using global variables to store state. Consider encapsulating these properties in a class or a more structured manner. |
||
| start_time = "" | ||
| end_time ="" | ||
| success_percentage = "" | ||
| total_conversion = "" | ||
|
|
||
| @app.route('/slack/events', methods=['POST']) #basica | ||
| def slack_event(): | ||
| ''' | ||
| This function is called when a slack event is triggered. | ||
| :return: | ||
| ''' | ||
| global name, start_time, end_time #i'm handling the variables awfully here. should change for better code practice | ||
| data = request.json | ||
| logging.info('Received data: %s', data) | ||
| # Extracting name | ||
| text = data['event']['text'] | ||
| name = text.split("http.host: ")[1].split("\n")[0].replace("'", "").split(" ")[0] | ||
| end_time = float(data['event']['event_ts']) | ||
| start_time = end_time - 3600 # 1 hour before | ||
| print("name: ", name) | ||
| print("start_time: ", start_time) | ||
| print("end_time: ", end_time) | ||
| fetch_from_newrelic(name,start_time,end_time) | ||
| return jsonify({'status': 'ok'}) | ||
|
|
||
| def fetch_from_newrelic(name, start_time, end_time): | ||
| ''' | ||
| This function fetches data from NR and populates the variables | ||
| :param name: | ||
| :param start_time: | ||
| :param end_time: | ||
| :return: | ||
|
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. Expressiveness — Use of unexplained string formats and API keys directly in code. Suggestion: Extract literals like API keys and URL to constants at the beginning of the file or to a config file/environment variables for clarity and easier maintenance. |
||
| ''' | ||
| # Convert Unix timestamp to datetime | ||
| start_time_dt = datetime.datetime.fromtimestamp(start_time, datetime.timezone(datetime.timedelta(hours=-7))) | ||
| end_time_dt = datetime.datetime.fromtimestamp(end_time, datetime.timezone(datetime.timedelta(hours=-7))) | ||
|
|
||
| # Convert datetime to string in ISO 8601 format which is handled by NR, excluding seconds | ||
| start_time_str = start_time_dt.strftime('%Y-%m-%d %H:%M-0700') | ||
| end_time_str = end_time_dt.strftime('%Y-%m-%d %H:%M-0700') | ||
|
|
||
| # The NRQL query with name and specific range of time | ||
| results_query = """{{actor {{ | ||
| account(id: SOME ACCOUNT ID) {{ | ||
| nrql( | ||
| query: "FROM Timer SELECT uniqueCount(CustomData_r1.artifact.id) as 'Unique Doc Count', count(CustomData_process.exit_code) as 'Conversion Attempts' WHERE Name = 'ConversionService.Complete' AND CustomData_process.exit_code IS NOT NULL AND CustomData_conversion.type.id = -1 AND CustomData_process.exit_code NOT IN (0, 4, 3328, 9, 11, 10, 59, 14) AND `CustomData_r1.tenant.name` = '{name}' LIMIT MAX FACET CustomData_process.exit_code, CustomData_message, CustomData_file.type SINCE '{start_time_str}' UNTIL '{end_time_str}'" | ||
| ) {{ | ||
| results | ||
| }} | ||
| }} | ||
| }} | ||
| }}""".format(name=name, start_time_str=start_time_str, end_time_str=end_time_str) | ||
|
|
||
| success_percentage_query = '''{{ | ||
| actor {{ | ||
| account(SOME ACCOUNT ID) {{ | ||
| nrql( | ||
| query: "SELECT latest(r1.source.name), latest(r1.ring.id), latest(cCnt) as 'Total Conversions', latest(sCnt) as 'Success Conversions', latest(sPer) as 'Success Percentage', latest(eCnt) as 'Error Conversoins', latest(ePer) as 'Error Percentage' FROM (SELECT count(CustomData_process.exit_code) as cCnt, filter(count(CustomData_process.exit_code), WHERE CustomData_process.exit_code IN (0, 4, 3328, 9, 11, 10, 59, 14)) as sCnt, percentage(count(*), WHERE CustomData_process.exit_code IN (0, 4, 3328, 9, 11, 10, 59, 14)) AS sPer, filter(count(CustomData_process.exit_code), WHERE CustomData_process.exit_code NOT IN (0, 4, 3328, 9, 11, 10, 59, 14)) as eCnt, percentage(count(*), WHERE CustomData_process.exit_code NOT IN (0, 4, 3328, 9, 11, 10, 59, 14)) AS ePer FROM Timer WHERE Name = 'ConversionService.Complete' AND CustomData_process.exit_code IS NOT NULL AND CustomData_conversion.type.id = -1 FACET CustomData_r1.tenant.name LIMIT MAX) JOIN ( SELECT r1.source.name, r1.ring.id, http.host FROM lookup(TenantLookup) ) ON `CustomData_r1.tenant.name` = http.host WHERE CustomData_r1.tenant.name = '{name}' FACET CustomData_r1.tenant.name LIMIT MAX SINCE '{start_time_str}' UNTIL '{end_time_str}'" | ||
| ) {{ | ||
| results | ||
| }} | ||
| }} | ||
| }} | ||
| }}'''.format(name=name, start_time_str=start_time_str, end_time_str=end_time_str) | ||
|
|
||
| headers = { | ||
| 'Accept': 'application/json', | ||
| 'Content-Type': 'application/json', | ||
| 'Api-Key' : "NRAK-16PL7JREKNJVUBNIMKVI0FMIICK" | ||
| } | ||
| SP_body = { | ||
| 'query': success_percentage_query, | ||
| 'accountID': 0000000 | ||
| } | ||
| results_body = { | ||
| 'query': results_query, | ||
| 'accountID': 0000000 | ||
| } | ||
| response = requests.post('https://api.newrelic.com/graphql', headers=headers, data = json.dumps(results_body)) | ||
| sp_response = requests.post('https://api.newrelic.com/graphql', headers=headers, data = json.dumps(SP_body)) | ||
| data = response.json() | ||
| sp_data = sp_response.json() | ||
| update_google_sheets(data, sp_data, end_time, name) | ||
|
|
||
|
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. Structure — Deep nesting makes the code hard to read and maintain. In this instance, loops and conditionals are nested deeply within the function. Suggestion: Refactor to reduce nesting. For example, handle error cases early and return or continue, reducing the need for deep conditional blocks. |
||
| return | ||
| def update_google_sheets(data, sp_data,end_time,name): | ||
| # Convert the timestamp to a datetime object | ||
| SPREADSHEET_ID = '1PQCxTDftKu0nPicnGmLHWJbbkUp0UAFYO1X3STN3izY' # Replace with your Spreadsheet ID | ||
| SHEET_NAME = 'Sheet1' | ||
| last_row = get_last_row(service, SPREADSHEET_ID, SHEET_NAME) | ||
|
|
||
| sheet = service.spreadsheets() | ||
| result = sheet.values().get(spreadsheetId=SPREADSHEET_ID, range='Sheet1!A:A').execute() | ||
| rows = result.get('values', []) | ||
| next_row = len(rows) + 2 # Next row after the last populated cell, plus two for the space | ||
|
|
||
|
Comment on lines
+69
to
+122
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. Functions — The function 'fetch_from_newrelic' is doing multiple tasks: constructing queries, converting data format, and making HTTP requests, which violates the single responsibility principle. Suggestion: Split this function into smaller functions, each handling one specific part of the task such as building the query, making the HTTP request, and processing the response. |
||
| dt_object = datetime.datetime.utcfromtimestamp(end_time) | ||
|
Comment on lines
+103
to
+123
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. DRY — HTTP request code is repeated for different queries within the 'fetch_from_newrelic' function, violating the DRY principle. Suggestion: Extract the HTTP request logic into a separate function that can be reused for different 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. Error Handling — HTTP requests might fail, but there's no specific error handling implemented for failed requests or erroneous data. Suggestion: Implement error handling for the HTTP request responses and verify their 'status' or 'data' content before processing them. |
||
| date = dt_object.strftime("%d-%b") | ||
| results = data['data']['actor']['account']['nrql']['results'] | ||
| total_conversions = round(sp_data['data']['actor']['account']['nrql']['results'][0]['Total Conversions'], 2) | ||
| success_percentage = round(sp_data['data']['actor']['account']['nrql']['results'][0]['Success Percentage'], 2) | ||
|
|
||
| # Prepare the last result | ||
| last_result_data = results[-1] | ||
| facet = last_result_data['facet'] | ||
| code, error_message, file_type = facet | ||
| conversion_attempts = str(last_result_data['Conversion Attempts']) | ||
| unique_doc_count = str(last_result_data['Unique Doc Count']) | ||
| total_conversions_str = str(int(total_conversions)) if total_conversions.is_integer() else str(round(total_conversions, 2)) | ||
| success_percentage_str = str(success_percentage) | ||
| last_result = [date, name, code, file_type, conversion_attempts, unique_doc_count, total_conversions_str, | ||
| success_percentage_str] | ||
| print(last_result) | ||
| print(last_row) | ||
| if last_row == last_result: | ||
| print("Data already processed, skipping.") | ||
| return | ||
| for result in results: | ||
| facet = result['facet'] | ||
| code, error_message, file_type = facet | ||
| conversion_attempts = result['Conversion Attempts'] | ||
| unique_doc_count = result['Unique Doc Count'] | ||
| row_data = [date, name, code, file_type, conversion_attempts, unique_doc_count, total_conversions,success_percentage] | ||
| body = { | ||
| 'values': [row_data] | ||
| } | ||
|
|
||
| range_str = f'Sheet1!A{next_row}:H{next_row}' | ||
| print(range_str) | ||
| result = service.spreadsheets().values().append(spreadsheetId=SPREADSHEET_ID, range=range_str, | ||
| valueInputOption='RAW', body=body).execute() | ||
|
|
||
| # Increment the row number | ||
| next_row += 1 | ||
|
|
||
| def get_last_row(service, SPREADSHEET_ID, SHEET_NAME): | ||
| range_str = f'{SHEET_NAME}!A:H' # Assuming the data is in columns A to H | ||
| result = service.spreadsheets().values().get(spreadsheetId=SPREADSHEET_ID, range=range_str).execute() | ||
| rows = result.get('values', []) | ||
| return rows[-1] if rows else None | ||
|
|
||
| if __name__ == '__main__': | ||
| app.run(port=3000) | ||
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.
Comments — The comment '#logging' is redundant as the following line of code clearly indicates that logging configuration is being set.
Suggestion: Remove the comment '#logging'.