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

c.user not defined fix #4574

Merged
merged 2 commits into from
Dec 7, 2018
Merged

c.user not defined fix #4574

merged 2 commits into from
Dec 7, 2018

Conversation

frafra
Copy link
Contributor

@frafra frafra commented Dec 6, 2018

Fixes #4247
This issue should have been ready fixed, but it seems that the patch did not covered all the cases.
This is not suppose to be the perfect patch for this issue, it seems a workaround for me, but this seems enough for my usage of CKAN.

More about it on: #4247 (comment)

Features:

  • includes tests covering changes
  • includes updated documentation
  • includes user-visible changes
  • includes API changes
  • includes bugfix for possible backport

Please [X] all the boxes above that apply

@smotornyuk
Copy link
Member

Just two small alterations:

  • Previously, pylons used to return empty string('') for any undefined value, so let's use the same, instead of None
  • We are replacing c global variable with g - they are pretty same, but latter is more Flask style, so could you switch to it as well?

@smotornyuk smotornyuk self-assigned this Dec 6, 2018
@frafra frafra force-pushed the c-user-not-defined branch 3 times, most recently from a9e5c91 to 6fada50 Compare December 7, 2018 09:44
@frafra
Copy link
Contributor Author

frafra commented Dec 7, 2018

Hi @smotornyuk, thank you for your suggestions.
I can use an empty string instead of None, but switching from c to g would not fix the issue. I tried to use g.user instead of c.user without any additional code, but it fails. I tried to setup g.user and then use c.user, but it still fails. The only way I found to avoid this issue is just to set c.user.

@smotornyuk
Copy link
Member

smotornyuk commented Dec 7, 2018

No, I'm perfectly ok with your solution and switching to g didn't suppose to fix the problem, because g and c are the same, as I said before. You just should use g instead of c in the feature, because c - it's how Pylons calls global context, and g - is a way of naming it from Flask. As we are migrating to Flask, I just want to use flask-style names for variable
https://github.com/ckan/ckan/wiki/Migration-from-Pylons-to-Flask#c-object

@frafra frafra force-pushed the c-user-not-defined branch from 6fada50 to 4db437b Compare December 7, 2018 11:04
@frafra
Copy link
Contributor Author

frafra commented Dec 7, 2018

Thank you for the clarification. I did not understand that they were equivalent and I was confused by the fact that using g.user is not enough, even if the previous patch set it.

I had to import g, as helpers.py uses c only.

@smotornyuk smotornyuk merged commit a4ec78e into ckan:master Dec 7, 2018
@frafra frafra deleted the c-user-not-defined branch December 10, 2018 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'_Globals' has no attribute 'user' : exception when using an IAuthenticator on CKAN 2.8.0
3 participants