You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/01/24 21:33:42 UTC
[GitHub] [incubator-superset] john-bodley opened a new pull request #9017:
[sip-15] Enabling SIP-15 by default
john-bodley opened a new pull request #9017: [sip-15] Enabling SIP-15 by default
URL: https://github.com/apache/incubator-superset/pull/9017
### CATEGORY
Choose one
- [ ] Bug Fix
- [ ] Enhancement (new features, refinement)
- [ ] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #9017: [sip-15]
Enabling SIP-15 by default
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9017: [sip-15] Enabling SIP-15 by default
URL: https://github.com/apache/incubator-superset/pull/9017#issuecomment-578341540
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9017?src=pr&el=h1) Report
> Merging [#9017](https://codecov.io/gh/apache/incubator-superset/pull/9017?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/f079b6dad072d6cca177cbf19da900b9e61dcc5e?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9017?src=pr&el=tree)
```diff
@@ 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 179
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9017?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9017?src=pr&el=footer). Last update [f079b6d...61935f2](https://codecov.io/gh/apache/incubator-superset/pull/9017?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] john-bodley commented on a change in pull
request #9017: [sip-15] Enabling SIP-15 by default
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9017: [sip-15] Enabling SIP-15 by default
URL: https://github.com/apache/incubator-superset/pull/9017#discussion_r370868110
##########
File path: superset/config.py
##########
@@ -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
Review 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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] john-bodley commented on issue #9017: [sip-15]
Enabling SIP-15 by default
Posted by GitBox <gi...@apache.org>.
john-bodley commented on issue #9017: [sip-15] Enabling SIP-15 by default
URL: https://github.com/apache/incubator-superset/pull/9017#issuecomment-581555115
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] john-bodley merged pull request #9017:
[sip-15] Enabling SIP-15 by default
Posted by GitBox <gi...@apache.org>.
john-bodley merged pull request #9017: [sip-15] Enabling SIP-15 by default
URL: https://github.com/apache/incubator-superset/pull/9017
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] john-bodley commented on a change in pull
request #9017: [sip-15] Enabling SIP-15 by default
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9017: [sip-15] Enabling SIP-15 by default
URL: https://github.com/apache/incubator-superset/pull/9017#discussion_r370887902
##########
File path: superset/config.py
##########
@@ -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
Review comment:
@mistercrunch @willbarrett do we want to take a passive (no end date) or aggressive (end date specified) approach?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] john-bodley commented on a change in pull
request #9017: [sip-15] Enabling SIP-15 by default
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9017: [sip-15] Enabling SIP-15 by default
URL: https://github.com/apache/incubator-superset/pull/9017#discussion_r370887902
##########
File path: superset/config.py
##########
@@ -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
Review comment:
@mistercrunch @willbarrett do we want to take a passive (no end date) or aggressive (end date specified) approach.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] willbarrett commented on a change in pull
request #9017: [sip-15] Enabling SIP-15 by default
Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #9017: [sip-15] Enabling SIP-15 by default
URL: https://github.com/apache/incubator-superset/pull/9017#discussion_r373720627
##########
File path: superset/config.py
##########
@@ -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
Review 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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] etr2460 commented on a change in pull request
#9017: [sip-15] Enabling SIP-15 by default
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #9017: [sip-15] Enabling SIP-15 by default
URL: https://github.com/apache/incubator-superset/pull/9017#discussion_r370865576
##########
File path: UPDATING.md
##########
@@ -46,6 +46,13 @@ now uses UTC for the tooltips and default placeholder timestamps (sans timezone)
have changed. FLASK_APP should be updated to `superset.app:create_app()` and Celery Workers
should be started with `--app=superset.tasks.celery_app:app`
+* [9017](https://github.com/apache/incubator-superset/pull/9017): `SIP_15_ENABLED` now
+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
Review comment:
sp nit: `for specifying how long chart`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] etr2460 commented on a change in pull request
#9017: [sip-15] Enabling SIP-15 by default
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #9017: [sip-15] Enabling SIP-15 by default
URL: https://github.com/apache/incubator-superset/pull/9017#discussion_r370865438
##########
File path: superset/config.py
##########
@@ -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
Review comment:
Should we add in a grace period date here? Maybe `2020-03-31` or something like that?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org