Final changes before end of Phase 1 Development #25
Conversation
| except ValueError as e: | ||
| # Invalid JSON, so don't remove old data | ||
| error_message = "Error saving data dictionary: {0}. Data was: {1}".format(e, record) | ||
| flash_error(error_message) |
There was a problem hiding this comment.
Awesome, but we probably want more explicit instructions, so that people know to avoid special characters and limit the amount of text?
There was a problem hiding this comment.
I had this happen when adding a data dictionary to a new dataset, no matter what I put into the fields. I think this is because the two stage dataset creation puts the data dictionary update logic into a weird state. I will look into it more tomorrow to see if there's a better fix.
There was a problem hiding this comment.
Sounds good. Check out the logs. Ultimately it seems to occur when CKAN attempts some text conversion before sending the data to the Datastore. It chokes on special characters that result from things like new line formatting, and it also seems to happen when there's too much text, which, for some reason, results in what might be a warning that includes special characters. Lots of implied question marks in the above. Happy hunting :)
|
Other than the error message for broken dictionaries, it looks good to me. |
tanderegg
changed the title
|
Latest commit has fixed the remaining outstanding issues, so I've removed the WIP label and we're good to merge. @sleitner could you do a quick review first? |
|
|
||
| if record: | ||
| # Cleanse of errant unicode characters | ||
| record = unicodedata.normalize('NFKD', record).encode('ascii', 'ignore') |
There was a problem hiding this comment.
Nice. Does this fix or just patch the problem for release? When ckan reads the data dictionary back from the database, does the formatting remain or does it get stripped out? Do very long entries still get cut off and have some special characters appended?
If not probably worth a comment somewhere.
There was a problem hiding this comment.
It normalizes non-ascii characters to their ascii equivalent, if one exists, so formatting should be kept. As far as I can tell, this fixes the long entry problem as well, I created one with 500 or so words that worked fine. New lines are still not being retained though, but that was existing behavior. Let me know if you know where that is happening.
There was a problem hiding this comment.
Not sure, but it sounds like this is good to go.
|
Nice work, Tim! |
Final changes before end of Phase 1 Development
|
Thanks! And thanks for writing easy to understand code :) |
@sleitner Can you take a look? There is one last pending issue to address, although we may end up skipping it, and it's minor. I'll remove the WIP when we've decided on that, but in the mean time you can go ahead and review the existing commits if you have bandwidth. Thanks!