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 dependencies #157

Closed
aitormagan opened this issue Oct 28, 2014 · 23 comments
Closed

Fix dependencies #157

aitormagan opened this issue Oct 28, 2014 · 23 comments

Comments

@aitormagan
Copy link

Each time the oauthlib library is changed, I have to change my code because the behaviour changes. So, can you fix the version of the libraries that you use?
At least you should fix the mayor number of the dependencies since when this number changes, the API often changes.

@ib-lundgren
Copy link
Member

@aitormagan sorry that oauthlib caused you problems. May I ask which project was affected so I can add it to my notifications list?

I'd rather not lock the version here when all tests pass. A better solution would be to add tests that would prevent this from happening in the future. Either new tests here in requests-oauthlib or in oauthlib itself depending on its nature.

Communicating changes downstream is a tricky problem and before I automate notifying all dependent libraries all the way down the stream I fear this things will happen. While hitting 1.0.0 and promising a stable API would help it won't really solve that much. If you have suggestions for improving the situation I'm all ears :)

@aitormagan
Copy link
Author

Hi! First of all I want to thank you for answering me so quickly! :)

The point I want to remark is that if you do not block the dependencies version and I continue using an old version (such as 0.4), when a SW is installed on a new system the latest version of the dependencies will be downloaded (even if the major number has changed). This can cause several problems because untestested dependencies can be downloaded. For example, if you upload the oauthlib library, you should run the tests for all the compatible versions of the request-oauthlib library since it has an upper unbounded dependency. So my suggestion is to lock to a specific dependency until 1.0.0 is ready and from then, ensure a proper semantic version. In my opinion, ensuring packaging consistency is preferred over packaging flexibility.

For example, in the past, I was using the enviroment variable DEBUG to enable OAuth2 working over HTTP. Nevertheless, when a new version of the oauthlib library was released, I've to change this variable even if I continued using the same version of request-oauthlib. As much as you can write good tests, there is always a posibility to break things.

@ib-lundgren
Copy link
Member

Indeed and when you intentionally break things it's hard to communicate that you have and the new approach. I think a good compromise in the future will be to add deprecation warnings for all features that go away and all APIs that change. Then let these warnings exist for at least one or two releases before making the breaking changes. A bit more hassle but should be far less disruptive.

@aitormagan
Copy link
Author

I think you have misunderstood what I said. When you change the way your framework works, I shouldn't be affeceted by your changes until I update the dependency.

@ib-lundgren
Copy link
Member

I'd be happy to pin requests-oauthlib to a version of oauthlib and find a better path to introduce breaking changes, possibly holding off entirely until oauthlib 1.0.

@aitormagan @sigmavirus24 @shazow @Lukasa what do you think would be less disruptive. 1) Locking to previous oauthlib version (0.6.3) and have people who have upgraded and experience issues downgrade/reinstall. 2) Try and quickly identify current issues and release a new version of oauthlib. Then pin to that version.

My sincerest apologies for the problems this caused, I will go to great lengths to ensure that next release + update will be smoother.

@Lukasa
Copy link
Member

Lukasa commented Nov 5, 2014

I think (2) is the only viable approach here. We need to avoid forcing users to downgrade oauthlib version if at all possible.

@aitormagan
Copy link
Author

I completly agree with @Lukasa! The second option is the best one!

About the problem of introducing breaking changes my suggestion is the following one:

  • Pin the dependencies ASAP without waiting for the 1.0 version.
  • If you introduce changes that impact on the functionality of the oauthlib, you can update requests-oauthlib, adapting the code, updating the dependencies and increasing the minor number.
  • If you introduce minor changes (such as bug fixes or refactors) on oauthlib you can update request-oauthlib, updating the dependices, adapting the code (if needed) and increasing the patch number.

@shazow
Copy link
Contributor

shazow commented Nov 5, 2014

I'd release a new minor requests-oauthlib which pins oauthlib to <0.7, then release a new major requests-oauthlib which unpins or pins to the next version. Depending how we want to manage the pinning.

@ib-lundgren
Copy link
Member

Did a little crawl and it turns out ~280 libraries directly or indirectly depend on oauthlib, of these ~250 go through requests-oauthlib. That is a whole lot more than I expected. I'll try identify which of these have been affected but it will take a while regardless of my automation plans.

@shazow @aitormagan would you mind pointing me to the affected projects so I can investigate them first before going over the ~70 other packages directly depending on requests-oauthlib. If the fire caused by oauthlib is caused by minor changes I can likely resolve them during or after my flight back from SFO to LON today.

If they can be fixed quickly (1-2 days) I'd say we aim for that, if not I think pinning to 0.6.3 and risking issues related to downgrading is better.

@aitormagan
Copy link
Author

My project is working perfectly now so you do not have to work about that. I was able to fix it by myself by adapting the code to the new changes.

@shazow
Copy link
Contributor

shazow commented Nov 5, 2014

@ib-lundgren The project I'm using requests-oauthlib is not open source. The issue was Google OAuth2, apparently the returned scope is empty string regardless what you ask for, so the "Scope changed" error is being raised every time with oauthlib>=0.7.

@ib-lundgren
Copy link
Member

@shazow interesting. I was not aware of any flow that returned an empty scope string. The spec does not explicitly address empty scope strings but my understanding is that they should be seen as scope changed from whatever you requested to no scope being authorized.

It never raised an error before on empty strings because the code lazily checked if scope as opposed to if scope is None. While not yet being very well documented (1) you can disable this warning by setting the env var OAUTHLIB_RELAX_TOKEN_SCOPE. If the scope has changed you can still access it using the ``scope_changedboolean property of the token. With the new scopes defined intoken.new_scope`.

I'd like to look at Googles argument for an empty string before deciding on whether to change anything but feeling its an error on their end atm. Which flow are you using? https://developers.google.com/accounts/docs/OAuth2#webserver ?

(1) Opened bug to inform users about these flags in the error msg oauthlib/oauthlib#292

@shazow
Copy link
Contributor

shazow commented Nov 6, 2014

In this case, the error is:

Warning: Scope has changed from "profile email https://www.googleapis.com/auth/analytics.readonly" to "".

I suspect this is related to the fact that the token is being refreshed?

I did see the "set this env var" workaround, but I'd prefer to avoid injecting env variables into my deployment process.

@ib-lundgren
Copy link
Member

Hrm can't reproduce using http://requests-oauthlib.readthedocs.org/en/latest/examples/real_world_example_with_refresh.html. Are you using an alternative application type than web app?

Regarding the env var. I know it's not the prettiest but set it from the Python app? An alternative could be to introduce flags but that is a bit awkward too.

@shazow
Copy link
Contributor

shazow commented Nov 6, 2014

Yea I'm doing something similar to that example, yes it's a web app. I have my own auto-refresh stuff just because it was broken when I first set it up. :P Maybe I should rewrite it.

@benjaminwhite
Copy link

I also encounter

Warning: Scope has changed from "..." to "".

Here's why it occurs (at least for me):

In oauth2_session.py, fetch_token and refresh_token both call (lines 199 and 257):

self._client.parse_request_body_response(r.text, scope=self.scope)

It's important to note that r.text does not contain details about scope.

Then in, oauthlib/oauth2/rfc6749/parameters.py, the r.text value is passed into a OAuth2Token object (375):

params = OAuth2Token(params, old_scope=scope)

The OAuth2Token searches for scope inside params (r.text's value) but finds none, so assigns self._new_scope to an empty set:

self._new_scope = set(utils.scope_to_list(params.get('scope', '')))

validate_token_parameters is then called on the OAuth2Token which is where the trouble occurs:

if params.scope_changed:
    message = 'Scope has changed from "{old}" to "{new}".'.format(
        old=params.old_scope, new=params.scope,
    )
    scope_changed.send(message=message, old=params.old_scopes, new=params.scopes)
    if not os.environ.get('OAUTHLIB_RELAX_TOKEN_SCOPE', None):
        w = Warning(message)
        w.token = params
        w.old_scope = params.old_scopes
        w.new_scope = params.scopes
        raise w

params.scope_changed compares new_scope to old_scope, and returns true because the old_scope has a value and new_scope doesn't.

This leads to the warning being raised.

The solution I found was to not pass scope into parse_request_body_response:

self._client.parse_request_body_response(r.text)

If you'd like, I can create a pull request.

@shazow
Copy link
Contributor

shazow commented Nov 15, 2014

@benjaminwhite You're amazing, thank you for taking the time to investigate that. :)

@ramusus
Copy link

ramusus commented Dec 15, 2014

@benjaminwhite, @shazow
Do you know is there any progress about solving this issue?
Warning: Scope has changed from "..." to "".
I also encountered this error, updated to last version of requests-oauthlib and it's still there.
Is there any fork? @benjaminwhite, did you say you can make a pull request?

@richgong
Copy link

+1

@akshar-raaj
Copy link
Contributor

In case somebody is still facing this issue, you can add os.environ['OAUTHLIB_RELAX_TOKEN_SCOPE'] = "1". I am not sure if this is the best solution, but it was a quick fix which worked for me.

@altryne
Copy link

altryne commented Mar 25, 2015

Guys I'm still hitting this issue as well! any progress on this?

wking added a commit to wking/google-form-templater that referenced this issue Apr 30, 2015
Work around:

  Warning: Scope has changed from "..." to "".

by pulling in this fix [1].  See previous discussion in [2,3].  Once
this gets released [4], we should update our requirements.txt to use a
tagged release.

[1]: oauthlib/oauthlib#323
[2]: requests/requests-oauthlib#157
[3]: requests/requests-oauthlib#173
[4]: https://pypi.python.org/pypi/oauthlib
@singingwolfboy
Copy link
Member

It appears that this issue was resolved a few years ago with the release of oauthlib 0.8.0. If the dependencies are still a problem for this project, please reopen this issue, or file a new issue and refer to this one.

@nbveeresh1995
Copy link

Thank you @benjaminwhite, 👍

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

No branches or pull requests