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/04/29 18:06:23 UTC
[GitHub] [incubator-superset] graceguo-supercat opened a new pull request #9685: [fix] reduce table metadata fetch for latest_partition check
graceguo-supercat opened a new pull request #9685:
URL: https://github.com/apache/incubator-superset/pull/9685
### CATEGORY
Choose one
- [x] Bug Fix
- [x] Enhancement (new features, refinement)
- [ ] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
In #9637 I added an API call to `/superset/extra_table_metadata` to check datasource table's metadata, if it has partition then show extra adhoc filter operator. But virtual table (when is_sqllab_view is True) doesn't have real table exists, so above metadata check will return server side error.
This PR is to add is_sqllab_view check before fetching table metadata.
### TEST PLAN
Manual test
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: #9637
- [ ] 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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #9685: [fix] reduce table metadata fetch for latest_partition check
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #9685:
URL: https://github.com/apache/incubator-superset/pull/9685#discussion_r417647315
##########
File path: superset/connectors/sqla/models.py
##########
@@ -581,6 +581,7 @@ def data(self) -> Dict:
d["main_dttm_col"] = self.main_dttm_col
d["fetch_values_predicate"] = self.fetch_values_predicate
d["template_params"] = self.template_params
+ d["is_sqllab_view"] = self.is_sqllab_view
Review comment:
It adds extra attribute in the datasource object. I am not sure how cache key use datasource. Is this a relevant cache key code?
https://github.com/apache/incubator-superset/blob/a6cedaaa879348aca49a520793bb20e63d152a1f/superset/common/query_object.py#L199
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #9685: [fix] reduce table metadata fetch for latest_partition check
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #9685:
URL: https://github.com/apache/incubator-superset/pull/9685#discussion_r417516105
##########
File path: superset-frontend/src/explore/components/controls/AdhocFilterControl.jsx
##########
@@ -125,9 +128,10 @@ export default class AdhocFilterControl extends React.Component {
);
}
}
- },
- // no error handler, in case of error do not show partition option
- );
+ })
+ .catch(error => {
+ console.log('fetch extra_table_metadata:', error.statusText);
Review comment:
can you add an eslint disable comment here to prevent the warning?
and maybe this should be a `console.error`?
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #9685: [fix] reduce table metadata fetch for latest_partition check
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9685:
URL: https://github.com/apache/incubator-superset/pull/9685#discussion_r417607393
##########
File path: superset/connectors/sqla/models.py
##########
@@ -581,6 +581,7 @@ def data(self) -> Dict:
d["main_dttm_col"] = self.main_dttm_col
d["fetch_values_predicate"] = self.fetch_values_predicate
d["template_params"] = self.template_params
+ d["is_sqllab_view"] = self.is_sqllab_view
Review comment:
@graceguo-supercat does this impact the cache key in anyway? If so we should probably add a note in `UPDATING.md` to inform others.
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #9685: [fix] reduce table metadata fetch for latest_partition check
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9685:
URL: https://github.com/apache/incubator-superset/pull/9685#discussion_r417672645
##########
File path: superset/connectors/sqla/models.py
##########
@@ -581,6 +581,7 @@ def data(self) -> Dict:
d["main_dttm_col"] = self.main_dttm_col
d["fetch_values_predicate"] = self.fetch_values_predicate
d["template_params"] = self.template_params
+ d["is_sqllab_view"] = self.is_sqllab_view
Review comment:
@graceguo-supercat I'm not exactly sure where this is used but I think it's unrelated to the cache. Sorry for the false alarm.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #9685: [fix] reduce table metadata fetch for latest_partition check
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #9685:
URL: https://github.com/apache/incubator-superset/pull/9685#discussion_r417647315
##########
File path: superset/connectors/sqla/models.py
##########
@@ -581,6 +581,7 @@ def data(self) -> Dict:
d["main_dttm_col"] = self.main_dttm_col
d["fetch_values_predicate"] = self.fetch_values_predicate
d["template_params"] = self.template_params
+ d["is_sqllab_view"] = self.is_sqllab_view
Review comment:
@john-bodley It adds extra attribute in the datasource object. I am not sure how cache key use datasource. Is this a relevant cache key code?
https://github.com/apache/incubator-superset/blob/a6cedaaa879348aca49a520793bb20e63d152a1f/superset/common/query_object.py#L199
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #9685: [fix] reduce table metadata fetch for latest_partition check
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #9685:
URL: https://github.com/apache/incubator-superset/pull/9685#discussion_r417524517
##########
File path: superset-frontend/src/explore/components/controls/AdhocFilterControl.jsx
##########
@@ -125,9 +128,10 @@ export default class AdhocFilterControl extends React.Component {
);
}
}
- },
- // no error handler, in case of error do not show partition option
- );
+ })
+ .catch(error => {
+ console.log('fetch extra_table_metadata:', error.statusText);
Review comment:
fixed. console.error is better.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org