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 dig import 2021 #66

Merged
merged 5 commits into from
Jun 25, 2021
Merged

Fix dig import 2021 #66

merged 5 commits into from
Jun 25, 2021

Conversation

dvbalen
Copy link
Contributor

@dvbalen dvbalen commented May 12, 2021

Account for Exceptions outside of CKAN validation
Pass DIG record to CKAN api for validation rather than error on missing field

Copy link
Collaborator

@tanderegg tanderegg left a comment

Choose a reason for hiding this comment

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

Looks good, two minor questions/comments, but fine to merge as-is.

#except ImportError: # pragma: no cover
# # If the custom exception can't be imported, use a more generic exception
# # This happens when ckan is not installed locally, like when running unit tests on travis.
# Invalid = Exception
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can problem just remove this rather than comment it out, no?

# Invalid = Exception

Invalid = Exception
NotFound = StopIteration
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we just use the actual Exceptions at this point, rather than alias them?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 85.0% when pulling 3d6db15 on dvb-cfpb:fix-dig-import-2021 into 646e0b7 on cfpb:master.

@dvbalen
Copy link
Contributor Author

dvbalen commented Jun 17, 2021

@tanderegg please review and merge

@@ -31,12 +31,12 @@ def upload(self):
dig = request.POST["file"].file
group = request.POST["group"]
rec, errors = make_rec(dig)
if errors:
if False: #errors:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add a comment explaining why we're skipping this, for now?

@tanderegg tanderegg merged commit 5f331f4 into cfpb:master Jun 25, 2021
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