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.

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

Fix crash on import_data(Dataset(), collect_failed_rows=True) #1381

Conversation

matthewhegarty
Copy link
Contributor

Problem

import_data() crashes when passed an empty dataset and collect_failed_rows is True.

See #1380

Solution

Add check for headers is None and create empty list if so.

Acceptance Criteria

  • Added unit tests.

@matthewhegarty matthewhegarty changed the title Fix crash on import_data(Dataset(), collect_failed_rows=True Fix crash on import_data(Dataset(), collect_failed_rows=True) Jan 20, 2022
@coveralls
Copy link

coveralls commented Jan 20, 2022

Coverage Status

Coverage increased (+0.001%) to 98.074% when pulling 15d38b4 on matthewhegarty:collect-failed-rows-crash into 0d4340f on django-import-export:main.

Copy link

@pokken-magic pokken-magic left a comment

Choose a reason for hiding this comment

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

I'm not sure enough of my feedback on this to approve or not on this one, let me know if it makes sense to =you.

import_export/results.py Show resolved Hide resolved
tests/core/tests/test_results.py Show resolved Hide resolved
Copy link

@pokken-magic pokken-magic left a comment

Choose a reason for hiding this comment

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

After our discussion I'm pretty comfortable with this one.

@matthewhegarty matthewhegarty merged commit 0ad13e0 into django-import-export:main Feb 17, 2022
@matthewhegarty matthewhegarty deleted the collect-failed-rows-crash branch February 17, 2022 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants