SEED-platform / seed

Standard Energy Efficiency Data (SEED) Platform™ is a web-based application that helps organizations easily manage data on the energy performance of large groups of buildings.
Other
107 stars 55 forks source link

Portfolio Manager import fails with TypeError #2536

Closed Ryoken closed 3 years ago

Ryoken commented 3 years ago

While running a Portfolio Manager import (via logging into ESPM) the app crashes while generating the mapping suggestions. See sentry report here: https://sentry.io/organizations/nrel/issues/2096132816/?project=1193349&query=&statsPeriod=90d#

Expected Behavior Site should not crash while importing properties.

Actual Behavior Site did crash.

Steps to Reproduce We were unable to reproduce this bug in testing.

Note PM may be not serving the data consistently and we need to do some extra logging to make sure that this is (or isn't) the case.

Ryoken commented 3 years ago

Since we could not reproduce, here is what we do know:

data_importer/views.py:1317 is creating import_file, which in several lines and without being modified is accessed successfully with if import_file.from_portfolio_manager:. However, the error is triggered when passing in import_file.first_row_columns, meaning the query was successful and the import file exists. first_row_columns is derived directly from the field cached_first_row, which means the field was blank. The cached_first_row function that appears to be called is from data_importer/tasks.py, which means either the parser is messing up or the import file has missing information.

adrian-lara commented 3 years ago

@Ryoken Just a heads up that Nick and I were chatting about this and we agree that the appropriate next step is to add some additional logging somewhere when we get unexpected data from ESPM. See the Note that he added to the main ticket text

adrian-lara commented 3 years ago

@Ryoken Any reason why this is no longer relevant? I'm assuming this hasn't happened recently? If that's the case, I'd agree that we can close unless this pops up again.

Ryoken commented 3 years ago

@adrian-lara Yeah, the original ticket's problem hasn't cropped up again. I can still keep this ticket open to add the logging mentioned in the comments, however, or move it into a new ticket. Preference?

adrian-lara commented 3 years ago

Ah yeah, adding the logging would be great. Let's just keep this ticket, and I'll throw a p-2 label on it

Ryoken commented 3 years ago

I spent a little more time trying to fix this bug and again could not reproduce it, so I boiled down my debug to a few statements I think will be able to identify the problem, should it crop up again.

RDmitchell commented 3 years ago

@macintoshpie -- not sure what to do with this one. No "monkey testing" for me to do.

macintoshpie commented 3 years ago

@RDmitchell no testing needed on your end -- we added additional logging to help debug this if it happens again later. Thanks!