[sip-15] Enabling SIP-15 by default #9017
Conversation
john-bodley
force-pushed
the
john-bodley--enable-sip-15-by-default
branch
3 times, most recently
from
3dbb2b7 to
2c5f733
Compare
| @@ -748,7 +748,7 @@ class CeleryConfig: # pylint: disable=too-few-public-methods | |||
| # | |||
| # Note if no end date for the grace period is specified then the grace period is | |||
| # indefinite. | |||
| SIP_15_ENABLED = False | |||
| SIP_15_ENABLED = True | |||
| SIP_15_GRACE_PERIOD_END: Optional[date] = None # exclusive | |||
There was a problem hiding this comment.
Should we add in a grace period date here? Maybe 2020-03-31 or something like that?
There was a problem hiding this comment.
@etr2460 I feel like indefinite is preferred to having a hard date, it would be troublesome if someone deployed this change and weren't aware that the grace period had already finished and thus this feature would serve as a hard transition.
There was a problem hiding this comment.
@mistercrunch @willbarrett do we want to take a passive (no end date) or aggressive (end date specified) approach?
There was a problem hiding this comment.
I'm not speaking for @mistercrunch , but I would recommend the aggressive approach. That would provide us a milestone for when the flag can be removed.
UPDATING.md
Outdated
| defaults to True which ensures that for all new SQL charts the time filter will behave | ||
| like [start, end). Existing deployments should either disable this feature to keep the | ||
| status quo or inform their users of this change prior to enabling the flag. The | ||
| `SIP_15_GRACE_PERIOD_END` option provides a mechanism for specifying long long chart |
There was a problem hiding this comment.
sp nit: for specifying how long chart
john-bodley
force-pushed
the
john-bodley--enable-sip-15-by-default
branch
from
2c5f733 to
dd835cc
Compare
john-bodley
force-pushed
the
john-bodley--enable-sip-15-by-default
branch
from
dd835cc to
61935f2
Compare
Codecov Report
@@ Coverage Diff @@
## master #9017 +/- ##
=======================================
Coverage 59.15% 59.15%
=======================================
Files 367 367
Lines 11686 11686
Branches 2866 2866
=======================================
Hits 6913 6913
Misses 4594 4594
Partials 179 179Continue to review full report at Codecov.
|
There was a problem hiding this comment.
this lgtm regardless of if the grace period is defaulted or not, but ping @mistercrunch and @willbarrett for their thoughts
|
I think for now the default should have SIP-15 enabled which will mean by default an infinite grace period. At a later date we can decide when we want to deprecate the old logic and thus hard-code a transition date. |
mistercrunch
added
🏷️ bot
CATEGORY
Choose one
SUMMARY
This PR enables SIP-15 by default.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
ADDITIONAL INFORMATION
REVIEWERS
to: @etr2460 @robdiciuccio @willbarrett