[GitHub] [incubator-superset] etr2460 commented on a change in pull request #9959: feat: Add additional information to datasets list API endpoint

[GitBox <gi...@apache.org>]

etr2460 commented on a change in pull request #9959:
URL: https://github.com/apache/incubator-superset/pull/9959#discussion_r439114934



##########
File path: superset/connectors/base/models.py
##########
@@ -51,6 +52,11 @@
 ]
 
 
+class DatasourceKind(Enum):

Review comment:
       Most enums i've seen in python have been in SHOUT_CASE, should we be consistent here?
   
   Also, I think if you have this extend `str` as well, then instead of returning `DatasourceKind.virtual.value` below you can return `str(DatasourceKind.virtual)`. see https://github.com/apache/incubator-superset/blob/7f6dbf838e4e527e640a002ce20bf5da1abf4a98/superset/errors.py#L24

##########
File path: superset/connectors/base/models.py
##########
@@ -99,6 +105,17 @@ def slices(self) -> RelationshipProperty:
             ),
         )
 
+    @property
+    def kind(self) -> str:
+        if self.sql:

Review comment:
       maybe `return DatasourceKind.virtual.value if self.sql else DatasourceKind.physical.value`?

##########
File path: superset/connectors/base/models.py
##########
@@ -377,7 +402,7 @@ def get_column(self, column_name: Optional[str]) -> Optional["BaseColumn"]:
 
     @staticmethod
     def get_fk_many_from_list(
-        object_list: List[Any], fkmany: List[Column], fkmany_class: Type, key_attr: str,
+        object_list: List[Any], fkmany: List[Column], fkmany_class: Type, key_attr: str

Review comment:
       hmm, why'd we lose the trailing comma here? Are you on the right version of black?

##########
File path: tests/datasets/api_tests.py
##########
@@ -23,6 +23,7 @@
 import yaml
 from sqlalchemy.sql import func
 
+import tests.test_app

Review comment:
       should this be included here? I don't see any code that references it




----------------------------------------------------------------
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