Closed shankari closed 10 months ago
@achasmita please investigate and fix ASAP
Ah wait, this may be because of the recent upgrade to a more recent version to fix the vulnerabilities?! But not sure how it is working on staging in that case...
Ah wait, this may be because of the recent upgrade to a more recent version to fix the vulnerabilities?! But not sure how it is working on staging in that case...
I think I figure out the problem before we did not have any column to discard in trajectory table but now we have discarded uuid for some config. so when i tried to discard a column I got the similar error.
after adding a column we get same error:
Traceback (most recent call last):
File "/usr/src/app/pages/data.py", line 88, in render_content
store_trajectories = update_store_trajectories(start_date_obj,end_date_obj)
File "/usr/src/app/pages/data.py", line 43, in update_store_trajectories
df = query_trajectories(start_date_obj,end_date_obj)
File "/usr/src/app/utils/db_utils.py", line 148, in query_trajectories
modified_columns = perm_utils.get_trajectories_columns(df.columns)
File "/usr/src/app/utils/permissions.py", line 103, in get_trajectories_columns
columns.discard(column)
AttributeError: 'Index' object has no attribute 'discard'
@achasmita
Can you please add an explanation of the change to the commit (or to the PR now)?
I understand that you were able to reproduce with this change, but I don't understand how your changes fix it.
The error is that columns
is an Index
https://pandas.pydata.org/pandas-docs/stable/reference/indexing.html
which does not have a discard
method.
But none of your changes seem to be related to this.
PLEASE INDICATE TESTING DONE
@achasmita
- Can you please add an explanation of the change to the commit (or to the PR now)? I understand that you were able to reproduce with this change, but I don't understand how your changes fix it. The error is that
columns
is anIndex
https://pandas.pydata.org/pandas-docs/stable/reference/indexing.html which does not have adiscard
method. But none of your changes seem to be related to this.- PLEASE INDICATE TESTING DONE
previously it was:
columns = list(data[0].keys())
as 'discard' cannot be used with list I changed it to:
columns = set(data[0].keys())
Testing is done.
@achasmita I see - so the issue is that the discard
method is only supported on sets and not on lists?
So you change the input to be a set instead of a list?
That seems like a really convoluted fix, and one which will be hard to maintain. Where did you get that code from?
The recommended approach to drop columns is to use drop
https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.DataFrame.drop.html
PLEASE INDICATE TESTING DONE
This means "please indicate the steps that you performed for testing, including their outputs" I have outlined current testing expectations earlier: https://github.com/e-mission/op-admin-dashboard/pull/63#issuecomment-1716187205 and below
previously while running this code
if 'data trajectories_columns_exclude' not in permissions:
permissions ['data_trajectories_columns_exclude'] = ['user_id']
I was getting error:
Traceback (most recent call last):
File "/usr/src/app/pages/data.py", line 88, in render_content
store_trajectories = update_store_trajectories(start_date_obj,end_date_obj)
File "/usr/src/app/pages/data.py", line 43, in update_store_trajectories
df = query_trajectories(start_date_obj,end_date_obj)
File "/usr/src/app/utils/db_utils.py", line 148, in query_trajectories
modified_columns = perm_utils.get_trajectories_columns(df.columns)
File "/usr/src/app/utils/permissions.py", line 103, in get_trajectories_columns
columns.discard(column)
AttributeError: 'Index' object has no attribute 'discard'
now there is no error.
for column in permissions.get("data_trips_columns_exclude", []):
columns.discard(column)
for column in permissions.get("data_uuids_columns_exclude", []):
columns.discard(column)
discard(column) was used for previous tables so I followed the same.
I am merging this now, since this is a showstopper and we want to have minimal changes.
But using a set and discard just seems wrong compared to df.drop
which we also use in other places in the codebase.
Just because it was done this way before doesn't mean that it is right, and it would be good to clean it up if it is clunky.
We don't just want code, we want nice code or at least, code that is getting nicer over time.
Fixed in #85
It does appear to work on staging, but does not work on production. I have tried:
Checking the cloudwatch logs, the error is