You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "mapledan (via GitHub)" <gi...@apache.org> on 2023/09/27 09:34:03 UTC
[GitHub] [superset] mapledan opened a new pull request, #25429: fix: the temporal x-axis results in a none time_range.
mapledan opened a new pull request, #25429:
URL: https://github.com/apache/superset/pull/25429
### SUMMARY
When the GENERIC_CHART_AXES feature is enabled, the `time_range` parameter in the API request is null.
Consequently, the `from_dttm` and `to_dttm` parameters rely on the `time_range` value, resulting in incorrect values being set for these parameters.
Specifically, this directly impacts the `from_dttm` and `to_dttm` variables in the Jinja as well.
Fixes #25301
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: the temporal x-axis results in a none time_range. [superset]
Posted by "mapledan (via GitHub)" <gi...@apache.org>.
mapledan commented on code in PR #25429:
URL: https://github.com/apache/superset/pull/25429#discussion_r1381282432
##########
superset/common/query_object_factory.py:
##########
@@ -99,6 +112,30 @@ def _process_row_limit(
)
return apply_max_row_limit(row_limit or default_row_limit)
+ def _process_time_range(
Review Comment:
Thanks for your review. I have added code comments and also unit tests.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: the temporal x-axis results in a none time_range. [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #25429:
URL: https://github.com/apache/superset/pull/25429#issuecomment-1768619432
@zhaoyongjie are you able to review the updates from @mapledan ? 🙏
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: the temporal x-axis results in a none time_range. [superset]
Posted by "yousoph (via GitHub)" <gi...@apache.org>.
yousoph merged PR #25429:
URL: https://github.com/apache/superset/pull/25429
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] sfirke commented on pull request #25429: fix: the temporal x-axis results in a none time_range.
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #25429:
URL: https://github.com/apache/superset/pull/25429#issuecomment-1737566982
Thanks for contributing this fix! I know from Slack that this issue affects many people. I tagged a few possible reviewers based on who has committed to that section of the codebase.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: the temporal x-axis results in a none time_range. [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25429:
URL: https://github.com/apache/superset/pull/25429#issuecomment-1791255401
@eschutho Ephemeral environment spinning up at http://34.220.132.241:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: the temporal x-axis results in a none time_range. [superset]
Posted by "mapledan (via GitHub)" <gi...@apache.org>.
mapledan commented on code in PR #25429:
URL: https://github.com/apache/superset/pull/25429#discussion_r1381282432
##########
superset/common/query_object_factory.py:
##########
@@ -99,6 +112,30 @@ def _process_row_limit(
)
return apply_max_row_limit(row_limit or default_row_limit)
+ def _process_time_range(
Review Comment:
Thanks for your review. I have added code comments and unit tests.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: the temporal x-axis results in a none time_range. [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on PR #25429:
URL: https://github.com/apache/superset/pull/25429#issuecomment-1791513210
/testenv up FEATURE_GENERIC_CHART_AXES=true FEATURE_ENABLE_TEMPLATE_PROCESSING=true
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] zhaoyongjie commented on pull request #25429: fix: the temporal x-axis results in a none time_range.
Posted by "zhaoyongjie (via GitHub)" <gi...@apache.org>.
zhaoyongjie commented on PR #25429:
URL: https://github.com/apache/superset/pull/25429#issuecomment-1737658961
@mapledan Thank you so much for the contribution, we should consider the multiple time filters in adhoc filters, I'll give you some snippet in this evening.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mapledan commented on pull request #25429: fix: the temporal x-axis results in a none time_range.
Posted by "mapledan (via GitHub)" <gi...@apache.org>.
mapledan commented on PR #25429:
URL: https://github.com/apache/superset/pull/25429#issuecomment-1739456142
I have addressed multiple cases related to the temporal filters.
Specifically:
1. Prioritize using the filter associated with the base_axis column.
2. When there is no base_axis, utilize the first temporal filter.
3. If `None` is passed as the time_range parameter, it will be corrected to NO_TIME_RANGE when calling the `get_since_until_from_time_range` function.
4. The processed time_range is not written back into the QueryObject.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: the temporal x-axis results in a none time_range. [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25429:
URL: https://github.com/apache/superset/pull/25429#issuecomment-1792810061
Ephemeral environment shutdown and build artifacts deleted.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: the temporal x-axis results in a none time_range. [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on PR #25429:
URL: https://github.com/apache/superset/pull/25429#issuecomment-1791252572
/testenv up FEATURE_GENERIC_CHART_AXES=true
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: the temporal x-axis results in a none time_range. [superset]
Posted by "betodealmeida (via GitHub)" <gi...@apache.org>.
betodealmeida commented on code in PR #25429:
URL: https://github.com/apache/superset/pull/25429#discussion_r1380756513
##########
superset/common/query_object_factory.py:
##########
@@ -99,6 +112,30 @@ def _process_row_limit(
)
return apply_max_row_limit(row_limit or default_row_limit)
+ def _process_time_range(
Review Comment:
This look good. It would be nice to define this as a staticmethod or separate function, so it's easier to test, and also add some unit tests. It's hard to understand what's going on here and what are the differnet contexts where this is running.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: the temporal x-axis results in a none time_range. [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25429:
URL: https://github.com/apache/superset/pull/25429#issuecomment-1791517723
@eschutho Ephemeral environment spinning up at http://54.71.30.64:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org