search

Pylons / pyramid

Pyramid - A Python web framework
https://trypyramid.com/
Other
3.97k stars 887 forks source link

cur_domain is in practice equivalent to '.' + cur_domain and therefor… #3586

Closed ericatkin closed 4 years ago

ericatkin commented 4 years ago

…e negates the effect of wild_domain

ericatkin commented 4 years ago

So, the lint failure is not related to this change. Do I need to do anything about that?

digitalresistor commented 4 years ago

Is this an issue that also exists on master? We'd prefer that pull requests are opened against master in that case and we as maintainers will do the work to back port the changes as necessary.

ericatkin commented 4 years ago

Yes, I believe so. I can work up a PR against master, but it is impossible for me to test in context because I haven't ported my codebase to the pyramid 2 semantics. This is so simple though, I think it will be ok. Eric

On Thu, May 28, 2020 at 2:04 PM Bert JW Regeer notifications@github.com wrote:

Is this an issue that also exists on master? We'd prefer that pull requests are opened against master in that case and we as maintainers will do the work to back port the changes as necessary.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Pylons/pyramid/pull/3586#issuecomment-635567106, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVJ6EAJLIK5326O37CNR7DRT27TNANCNFSM4NNLK5IA .

mmerickel commented 4 years ago

This change is bw-incompat in the cookie format so I think I'm unlikely to want to release it in the 1.10 series. We'll have to decide whether to backport or not after it makes it into master.

ericatkin commented 4 years ago

What do you mean by backward incompatible? Do you mean it is important to preserve the current (broken) behavior? Perhaps I don't understand all the implications of the wild_domain arg, but it appears to be useless (setting it to False has no practical effect) without this change and I don't see value in preserving its uselessness, though that would be bw compatible :) Eric

On Thu, May 28, 2020 at 2:16 PM Michael Merickel notifications@github.com wrote:

This change is bw-incompat in the cookie format so I think I'm unlikely to want to release it in the 1.10 series. We'll have to decide whether to backport or not after it makes it into master.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Pylons/pyramid/pull/3586#issuecomment-635581770, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVJ6EFTMAPWASVAABKYVE3RT3BDNANCNFSM4NNLK5IA .

digitalresistor commented 4 years ago

You are correct that currently Pyramid's handling of wild_domain is broken. However we take backwards compatibility seriously, and if we make a change to this now, we may unwittingly break peoples deployments using Pyramid that are depending on the current cookie handling behaviour (including its brokenness).

We tend to weigh the potential impacts on making changes to our long support branches that may impact current users.

Once we have the change on master, and we think through all of the ramifications, we will make a decision on the best way to make sure we don't catch people unawares.

ericatkin commented 4 years ago

Makes sense, and this is something I really appreciate about the pyramid folks. I just wanted to make sure I understand. I'll send a PR on master shortly. Should I close this one? Thanks, Eric

On Thu, May 28, 2020 at 2:27 PM Bert JW Regeer notifications@github.com wrote:

You are correct that currently Pyramid's handling of wild_domain is broken. However we take backwards compatibility seriously, and if we make a change to this now, we may unwittingly break peoples deployments using Pyramid that are depending on the current cookie handling behaviour (including its brokenness).

We tend to weigh the potential impacts on making changes to our long support branches that may impact current users.

Once we have the change on master, and we think through all of the ramifications, we will make a decision on the best way to make sure we don't catch people unawares.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Pylons/pyramid/pull/3586#issuecomment-635589066, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVJ6EGQED56DQF2YWDESZTRT3CMHANCNFSM4NNLK5IA .

digitalresistor commented 4 years ago

Feel free to close/refer to this one when you make your PR against master. You might find that most of this patch applies against master, but don't quote me on that!

ericatkin commented 4 years ago

See PR #3587

mmerickel commented 4 years ago

Thanks for clarifying @bertjwregeer.