From 8b5281d1fd263b0e27f547beb3640931bb531c70 Mon Sep 17 00:00:00 2001 From: "AARETE\\agupta" Date: Tue, 4 Jun 2024 14:40:03 -0500 Subject: [PATCH 1/7] For UI 0 - Wrote comments that identify hardcodings and provide recommendations on a potential better approach. --- streamlit/interface_0.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/streamlit/interface_0.py b/streamlit/interface_0.py index f638209..e0c0e63 100644 --- a/streamlit/interface_0.py +++ b/streamlit/interface_0.py @@ -14,10 +14,10 @@ from io import StringIO, BytesIO from sf_conn import get_client_names, insert_upload_logs from constants import USER_LIST -create_batch_url = 'https://lfksus2t62.execute-api.us-east-2.amazonaws.com/dev/create-batch' +create_batch_url = 'https://lfksus2t62.execute-api.us-east-2.amazonaws.com/dev/create-batch' # RECOMMENDATION - Store in Constants or AWS Secrets Manager -REDIRECT_URI = 'https://doczydev.aarete.com:8500' -user_list = USER_LIST +REDIRECT_URI = 'https://doczydev.aarete.com:8500' +user_list = USER_LIST # RECOMMENDATION - Unsafe in Constants as well, should be fetched from Snowflake st.set_page_config(layout = "wide") # # Sidebar contents # with st.sidebar: @@ -37,17 +37,16 @@ try: util.setup_page(REDIRECT_URI) except Exception as e: st.write(f"SSO Failed = {e}") - st.session_state['user_info'] = {'mail': 'maamseek@aarete.com', 'displayName': 'Mayank Aamseek'} -try: + st.session_state['user_info'] = {'mail': 'maamseek@aarete.com', 'displayName': 'Mayank Aamseek'} # RECOMMENDATION - Remove Line after development. c1.write(f"User: **{st.session_state.user_info['displayName']}**") user_mail = st.session_state.user_info['mail'] -except KeyError as e: +except KeyError as e: # RECOMMENDATION - Not Working as Expected. Unreachable Code because the above Exception handles everything st.write("Session Expired.") st.stop() -s3_client = boto3.client('s3', +s3_client = boto3.client('s3', # RECOMMENDATION - We can make use of AWS Access Key for enabling this in local testing. Will help a lot. region_name="us-east-2", ) @@ -84,7 +83,7 @@ df['Contract Name'] = file_list file_names = [] buttons = st.columns([0.4, 0.4, 0.2]) with buttons[1]: - if st.button("Create Batch"): + if st.button("Create Batch"): # RECOMMENDATION - Use on-click to handle everything below this. Better in terms of re-running code. myobj = { "client-bucket-name": client_bucket } response = requests.post(create_batch_url, json = myobj) @@ -113,7 +112,7 @@ with buttons[1]: st.write(f"{batch_id} created") st.write(f"Files uploaded to s3://{client_bucket}/{landing_zone}{batch_id}") - + # RECOMMENDATION - Rethink about how to best manage cache # @st.cache_data # def convert_df(df): # return df.to_csv(index=False).encode('utf-8') From 8e514579490f63b670939b3d77d93d28180d6b43 Mon Sep 17 00:00:00 2001 From: "AARETE\\agupta" Date: Tue, 4 Jun 2024 15:04:31 -0500 Subject: [PATCH 2/7] For UI 1 - Wrote comments that identify hardcodings and provide recommendations on a potential better approach. --- streamlit/interface_1.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/streamlit/interface_1.py b/streamlit/interface_1.py index 3d3a695..0ed5a36 100644 --- a/streamlit/interface_1.py +++ b/streamlit/interface_1.py @@ -14,7 +14,7 @@ from constants import USER_LIST, DOCZY_PIPELINE_URL_DEV doczy_pipeline = DOCZY_PIPELINE_URL_DEV REDIRECT_URI = 'https://doczydev.aarete.com:8501' -user_list = USER_LIST +user_list = USER_LIST # RECOMMENDATION - Store in Snowflake probably st.set_page_config(layout = "wide") # # Sidebar contents # with st.sidebar: @@ -34,7 +34,7 @@ try: util.setup_page(REDIRECT_URI) except: st.write("SSO Failed") - st.session_state['user_info'] = {'mail': 'maamseek@aarete.com', 'displayName': 'Mayank Aamseek'} + st.session_state['user_info'] = {'mail': 'maamseek@aarete.com', 'displayName': 'Mayank Aamseek'} # RECOMMENDATION - Remove after dev phase try: c1.write(f"User: **{st.session_state.user_info['displayName']}**") user_mail = st.session_state.user_info['mail'] @@ -114,7 +114,7 @@ if st.button("Read the contracts from Path"): df['Contract Name'] = file_list # df['Request ID'] = range(len(file_list)) # df['Contract ID'] = file_list - df['Unique Key'] = a + df['Unique Key'] = a # RECOMMENDATION - Use a List or Dictionary to make it automated based on a given list of Group Nos. df['Pricing Before Carveouts'] = b df['Contract Related'] = c df['Provider'] = d @@ -123,16 +123,16 @@ if st.button("Read the contracts from Path"): df['Carveout Methodology'] = g dir_path = os.path.dirname(os.path.realpath(__file__)) print(f'DEBUGGING: PWD= {dir_path}') - df.to_csv('temp1.csv', index=False) + df.to_csv('temp1.csv', index=False) add_vertical_space(1) -df2 = pd.read_csv('temp1.csv') +df2 = pd.read_csv('temp1.csv') # RECOMMENDATION - Breaks initially when temp1.csv does not exist in the files. Should by default create an empty table so that it never breaks edited_df = st.data_editor(df2) edited_df['REQUEST_USER'] = user_mail edited_df['LATEST_FLAG BOOLEAN'] = True edited_df['PIPELINE_KICKOFF_DATETIME'] = datetime.now().strftime("%Y-%m-%d %H:%M:%S") -edited_df['REQUEST_DATETIME'] = datetime.now().strftime("%Y-%m-%d %H:%M:%S") +edited_df['REQUEST_DATETIME'] = datetime.now().strftime("%Y-%m-%d %H:%M:%S") # RECOMMENDATION - Timezone Error on Server-Side. Works Properly Locally @st.cache_data def convert_df(df): @@ -148,7 +148,7 @@ additional_info.loc[0] = [client, batch_id, st.session_state.user_info['mail'], st.write(additional_info) contract_list = [] -for index, row in edited_df.iterrows(): +for index, row in edited_df.iterrows(): # RECOMMENDATION - Very HardCode right now. Again use a 2D Array or Dictionary for best practice group_list = [] if row['Unique Key']: group_list.append('Unique Key') @@ -183,7 +183,7 @@ buttons = st.columns([0.8, 0.2]) with buttons[0]: st.download_button("Download Table", csv, "file.csv", "text/csv", key='download-csv') with buttons[1]: - if st.button("Run Doczy.AI Pipeline"): + if st.button("Run Doczy.AI Pipeline"): # RECOMMENDATION - Use on_click() to deal with button pressing # csv_buf = StringIO() # additional_info.to_csv(csv_buf, header=True, index=False) # csv_buf.seek(0) From 5bd2ce5972dcada2275c8217d24f58c7cecd5750 Mon Sep 17 00:00:00 2001 From: "AARETE\\agupta" Date: Wed, 5 Jun 2024 10:15:14 -0500 Subject: [PATCH 3/7] For UI 2 - Wrote comments that identify hardcodings and provide recommendations on a potential better approach. --- streamlit/interface_2.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/streamlit/interface_2.py b/streamlit/interface_2.py index ed8c494..e64e749 100644 --- a/streamlit/interface_2.py +++ b/streamlit/interface_2.py @@ -21,7 +21,7 @@ import re from sf_conn import get_snowflake_conn REDIRECT_URI = 'https://doczydev.aarete.com:8502' -user_list = USER_LIST +user_list = USER_LIST # RECOMMENDATION - AWS Secrets Manager or Snowflake st.set_page_config(layout = "wide") # Sidebar contents @@ -39,10 +39,10 @@ with st.sidebar: _,c1= st.columns([5,1]) try: - util.setup_page(REDIRECT_URI) + util.setup_page(REDIRECT_URI) # RECOMMENDATION - Not secure enough except: st.write("SSO Failed") - st.session_state['user_info'] = {'mail': 'maamseek@aarete.com', 'displayName': 'Mayank Aamseek'} + st.session_state['user_info'] = {'mail': 'maamseek@aarete.com', 'displayName': 'Mayank Aamseek'} # RECOMMENDATION - Remove after dev phase try: c1.write(f"User: **{st.session_state.user_info['displayName']}**") user_mail = st.session_state.user_info['mail'] @@ -54,21 +54,21 @@ except KeyError as e: try: conn = get_snowflake_conn('STG') cur = conn.cursor() - query = 'select * from "TRAINING_DATA_RAW"' + query = 'select * from "TRAINING_DATA_RAW"' # RECOMMENDATION - Move all queries to another file like constants cur.execute(query) field_values = pd.DataFrame.from_records(iter(cur), columns=[x[0] for x in cur.description]) - field_values['Document_Name'] = field_values['DOCUMENT_NAME'] + field_values['Document_Name'] = field_values['DOCUMENT_NAME'] # RECOMMENDATION - Use String Matching or something less hardcoded except: # field_values = pd.read_csv('contract_field_values.csv', encoding='unicode_escape', skipinitialspace=True) # field_values = field_values.loc[:, ~field_values.columns.str.contains('Unnamed:')] st.write("Conn failed, unable to fetch data from training data table in Snowflake") try: - query = 'select * from "PROMPT_CONFIG"' + query = 'select * from "PROMPT_CONFIG"' # RECOMMENDATION - Move query somewhere else cur.execute(query) fields = pd.DataFrame.from_records(iter(cur), columns=[x[0] for x in cur.description]) - fields.rename(columns={'FIELD_DESC': 'Field Name'}, inplace = True) + fields.rename(columns={'FIELD_DESC': 'Field Name'}, inplace = True) # RECOMMENDATION - Package into less hardcoded function or handle in DB fields.rename(columns={'PROMPT': 'Interrogation Question?'}, inplace = True) fields.rename(columns={'GROUP_ID': 'PRIORITY'}, inplace = True) fields.rename(columns={'FIELD_NAME': 'SF_DB_COL_NAME'}, inplace = True) @@ -103,13 +103,13 @@ field_row = st.columns([0.2, 0.7, 0.1]) with field_row[0]: st.write("**Field Group**") with field_row[1]: - field_group = st.selectbox('Field Group',('Unique Key', 'Contract Related', 'Pricing Before Carveouts - I' + field_group = st.selectbox('Field Group',('Unique Key', 'Contract Related', 'Pricing Before Carveouts - I' # RECOMMENDATION - Get from a separate list or snowflake , 'Pricing Before Carveouts - II', 'Carveout Indicator, Code Type and Code #s - I' , 'Carveout Indicator, Code Type and Code #s - II', 'Carveout Indicator, Code Type and Code #s - III' , 'Optimize Carving Indic.', 'Carveout Method - I', 'Carveout Method - II', 'Provider' , 'Timeline'), label_visibility = "collapsed") -if field_group == 'Unique Key': +if field_group == 'Unique Key': # RECOMMENDATION - Use a Dictionary for this mapping fields = fields[fields['PRIORITY'] == 'A'] elif field_group == 'Contract Related': fields = fields[fields['PRIORITY'] == 'C'] @@ -146,7 +146,7 @@ elif field_group == 'Timeline': fields = fields[fields['PRIORITY'] == 'E'] if st.button("Show Results"): - query = 'select * from "DOCZY_PIPELINE_RAW_OUTPUT"' + query = 'select * from "DOCZY_PIPELINE_RAW_OUTPUT"' # RECOMMENDATION - Move query somewhere else cur.execute(query) df2 = pd.DataFrame.from_records(iter(cur), columns=[x[0] for x in cur.description]) From a21b5e46a52e82ca7f63847e2571fcf7b6a4bf1b Mon Sep 17 00:00:00 2001 From: Pratham Soni Date: Wed, 5 Jun 2024 12:00:54 -0500 Subject: [PATCH 4/7] pratham hardcoding comments --- streamlit/interface_0.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/streamlit/interface_0.py b/streamlit/interface_0.py index e0c0e63..217b31f 100644 --- a/streamlit/interface_0.py +++ b/streamlit/interface_0.py @@ -83,6 +83,7 @@ df['Contract Name'] = file_list file_names = [] buttons = st.columns([0.4, 0.4, 0.2]) with buttons[1]: + #Use the st.session_state to handle the changes that are made when create batch button is pressed if st.button("Create Batch"): # RECOMMENDATION - Use on-click to handle everything below this. Better in terms of re-running code. myobj = { "client-bucket-name": client_bucket } @@ -98,11 +99,16 @@ with buttons[1]: landing_zone = 'contracts_landing_zone' else: st.write("Failed") + #Consider using a batch upload and upload all the files to the s3 client bucket at the same time instead of a for loop to improve effeciency for uploaded_file in file_list: + #Using BytesIO seems unnecessary, and will cause processing delays, instead handle the stream directly stringio = BytesIO(uploaded_file.getvalue()) stringio.seek(0) s3_client.put_object(Bucket=client_bucket, Body=stringio.getvalue(), Key= landing_zone+batch_id+'/'+str(uploaded_file.name)) + #s3_client.put_object(Bucket=client_bucket, Body=uploaded_file, + # Key=landing_zone+batch_id+'/'+str(uploaded_file.name)) + # TODO: Test this insert function with snowflake upload_log = insert_upload_logs(batch_id, client, str(uploaded_file.name), datetime.now().strftime("%Y-%m-%d %H:%M:%S"), user_mail) From 9864a58a99c988e43346aff1dda396c7db81f00f Mon Sep 17 00:00:00 2001 From: Pratham Soni Date: Wed, 5 Jun 2024 14:19:21 -0500 Subject: [PATCH 5/7] pratham comments hardcoding in interface 1 and 2 --- streamlit/interface_2.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/streamlit/interface_2.py b/streamlit/interface_2.py index e64e749..df79a61 100644 --- a/streamlit/interface_2.py +++ b/streamlit/interface_2.py @@ -146,6 +146,8 @@ elif field_group == 'Timeline': fields = fields[fields['PRIORITY'] == 'E'] if st.button("Show Results"): + #Might be beneficial to have some error handling here for if query execution isn't successful + query = 'select * from "DOCZY_PIPELINE_RAW_OUTPUT"' # RECOMMENDATION - Move query somewhere else cur.execute(query) df2 = pd.DataFrame.from_records(iter(cur), columns=[x[0] for x in cur.description]) @@ -159,6 +161,8 @@ df2 = pd.read_csv('temp2.csv') df2['Imputed Value'] = '' edited_df = st.data_editor(df2) + + @st.cache_data def convert_df(df): return df.to_csv(index=False).encode('utf-8') From f131f436cc1fc27040f4d762b80203e07207128a Mon Sep 17 00:00:00 2001 From: "AARETE\\agupta" Date: Wed, 5 Jun 2024 16:12:39 -0500 Subject: [PATCH 6/7] Few more insights added to comments after discussion with Umang --- streamlit/interface_2.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/streamlit/interface_2.py b/streamlit/interface_2.py index df79a61..93f1238 100644 --- a/streamlit/interface_2.py +++ b/streamlit/interface_2.py @@ -82,7 +82,7 @@ except Exception as e: s3_client = boto3.client('s3', region_name="us-east-2" ) -bucket = 'doczy-dev-infra-textract' +bucket = 'doczy-dev-infra-textract' # needs to be dynamic based on Client Name and Batch ID objects = s3_client.list_objects_v2(Bucket=bucket, Prefix="training-data/") file_list = [] for obj in objects['Contents']: @@ -146,9 +146,9 @@ elif field_group == 'Timeline': fields = fields[fields['PRIORITY'] == 'E'] if st.button("Show Results"): - #Might be beneficial to have some error handling here for if query execution isn't successful + #Might be beneficial to have some error handling here for if query execution isn't successful - Use try and catch to handle error - query = 'select * from "DOCZY_PIPELINE_RAW_OUTPUT"' # RECOMMENDATION - Move query somewhere else + query = 'select * from "DOCZY_PIPELINE_RAW_OUTPUT"' # RECOMMENDATION - Move query somewhere else - needs to have batch ID and client name cur.execute(query) df2 = pd.DataFrame.from_records(iter(cur), columns=[x[0] for x in cur.description]) From de0a672f1bdd877cf5e40ce54b1d7ad2ef9cc019 Mon Sep 17 00:00:00 2001 From: Michael McGuinness Date: Fri, 27 Sep 2024 13:13:04 +0100 Subject: [PATCH 7/7] try --- streamlit/interface_0.py | 1 + 1 file changed, 1 insertion(+) diff --git a/streamlit/interface_0.py b/streamlit/interface_0.py index 4ac0f71..0e37a1f 100644 --- a/streamlit/interface_0.py +++ b/streamlit/interface_0.py @@ -79,6 +79,7 @@ try: except Exception as e: st.write(f"SSO Failed = {e}") st.session_state['user_info'] = {'mail': 'maamseek@aarete.com', 'displayName': 'Mayank Aamseek'} # RECOMMENDATION - Remove Line after development. +try: c1.write(f"User: **{st.session_state.user_info['displayName']}**") user_mail = st.session_state.user_info['mail'] except KeyError as e: # RECOMMENDATION - Not Working as Expected. Unreachable Code because the above Exception handles everything