replace usage of unicode with unicode_safe
Fix #298
hey @wardi what's your opinion over this? Thanks
There is a number of places that can be changed as well(even though they do not affect the logic and can be ignored):
@smotornyuk thanks for your suggestion and can you please clarify what you mean by or replaced with unicode_safe for backward compatibility
Thanks
hey @smotornyuk does my latest commit seems ok ? Thanks
or replaced with unicode_safe for backward compatibility
If you just drop it, it's ok for new schemas. But every person who is using unicode validator will get an error. Actually, I like it, because in that way we can be sure that people will replace unicode with unicode_safe.
But, if you want to hide this change (I'll explain, why you may choose this option) and to make your PR compatible with existing schemas(i.e, unicode in schema still can be used), you can write something like
if name == 'unicode':
name = 'unicode_safe'
does my latest commit seems ok ?
Except for backward compatibility - yes. But you've introduced one issue. the unicode_safe validator was added in CKAN v2.8, while this repo guarantees support for CKAN starting from v2.6.
In v2.6 and v2.7 you still have to use unicode validator. This means you have to replace this:
if name == 'unicode':
return six.text_type
with something like
if name == 'unicode' and not ckantoolkit.check_ckan_version('2.8'):
return six.text_type
I'd be happy to add a copy of unicode_safe to ckanext-scheming so it could be used on 2.7. I'm indifferent about 2.6 as that's not an officially supported release anymore
Hey Ian, is there anything else that can be done on this PR? Thanks
Hi everyone. May I ask, what's gonna happen with this? I am trying to migrate to 2.10 and am experiencing some trouble with this. I guess once more ppl migrate, more will be affected? There are no more logs for the failed tests so I am not sure what the issue is. If it's something I can help fix, I'm up for it!
I don't see a way to re-run tests from the web UI, @TomeCirun if you want to merge again we should be able to see the reason for the test failures, or @ChristianF88 if you like you could create another PR based on this branch
I don't see a way to re-run tests from the web UI, @TomeCirun if you want to merge again we should be able to see the reason for the test failures, or @ChristianF88 if you like you could create another PR based on this branch
Hi @TomeCirun, I'm not sure how to interpret the thumbs up. Are you gonna merge again? Cheers