Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ disable=
missing-docstring,
too-many-lines,
duplicate-code,
unspecified-encoding,
Copy link
Copy Markdown
Member Author

@villebro villebro Aug 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't necessarily agree with this rule. While we could add encoding="utf-8" to all text file opens, this would likely break compatibility with Windows which defaults to cp-1252 (not officially supported, but I know some parties in the community are running Superset on Windows). Also, adding encoding=locale.getpreferredencoding() feels pointless. So I recommend we disable this rule for now.

# re-enable once this no longer raises false positives
too-many-instance-attributes

[REPORTS]

Expand Down
2 changes: 1 addition & 1 deletion requirements/testing.in
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ openpyxl
parameterized
pyfakefs
pyhive[presto]>=0.6.3
pylint==2.6.0
pylint==2.10.2
pytest
pytest-cov
statsd
Expand Down
6 changes: 3 additions & 3 deletions requirements/testing.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SHA1:5bfcfb5d0ab31dd532ce58caa2aab91d6807b123
# SHA1:59e47200215ca4695f09e03a773e1a6f310f78da
#
# This file is autogenerated by pip-compile-multi
# To update, run:
Expand All @@ -11,7 +11,7 @@
# via -r requirements/base.in
appnope==0.1.2
# via ipython
astroid==2.6.6
astroid==2.7.2
# via pylint
backcall==0.2.0
# via ipython
Expand Down Expand Up @@ -71,7 +71,7 @@ pyhive[hive,presto]==0.6.4
# via
# -r requirements/development.in
# -r requirements/testing.in
pylint==2.9.6
pylint==2.10.2
# via -r requirements/testing.in
pytest==6.2.4
# via
Expand Down
4 changes: 2 additions & 2 deletions superset/annotation_layers/annotations/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ def put( # pylint: disable=arguments-differ
try:
new_model = UpdateAnnotationCommand(g.user, annotation_id, item).run()
return self.response(200, id=new_model.id, result=item)
except (AnnotationNotFoundError, AnnotationLayerNotFoundError) as ex:
except (AnnotationNotFoundError, AnnotationLayerNotFoundError):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm always happy to see Pylint is continuing to evolve it's logic.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was actually really surprised this wasn't caught by older versions

return self.response_404()
except AnnotationInvalidError as ex:
return self.response_422(message=ex.normalized_messages())
Expand Down Expand Up @@ -438,7 +438,7 @@ def delete( # pylint: disable=arguments-differ
try:
DeleteAnnotationCommand(g.user, annotation_id).run()
return self.response(200, message="OK")
except AnnotationNotFoundError as ex:
except AnnotationNotFoundError:
return self.response_404()
except AnnotationDeleteFailedError as ex:
logger.error(
Expand Down
2 changes: 1 addition & 1 deletion superset/annotation_layers/annotations/commands/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def run(self) -> Model:
return annotation

def validate(self) -> None:
exceptions: List[ValidationError] = list()
exceptions: List[ValidationError] = []
Copy link
Copy Markdown
Member Author

@villebro villebro Aug 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this made me chuckle. I don't remember which linter and when, but didn't some linter up until fairly recently complain about using [] and recommend using list() instead? 😆 See use-list-literal (R1734) and use-dict-literal (R1735) in http://pylint.pycqa.org/en/latest/technical_reference/features.html
(FYI: I much prefer [] and {} over list() and dict() respectively, so I'm really happy about this change)

layer_id: Optional[int] = self._properties.get("layer")
start_dttm: Optional[datetime] = self._properties.get("start_dttm")
end_dttm: Optional[datetime] = self._properties.get("end_dttm")
Expand Down
2 changes: 1 addition & 1 deletion superset/annotation_layers/annotations/commands/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def run(self) -> Model:
return annotation

def validate(self) -> None:
exceptions: List[ValidationError] = list()
exceptions: List[ValidationError] = []
layer_id: Optional[int] = self._properties.get("layer")
short_descr: str = self._properties.get("short_descr", "")

Expand Down
4 changes: 2 additions & 2 deletions superset/annotation_layers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def delete(self, pk: int) -> Response:
try:
DeleteAnnotationLayerCommand(g.user, pk).run()
return self.response(200, message="OK")
except AnnotationLayerNotFoundError as ex:
except AnnotationLayerNotFoundError:
return self.response_404()
except AnnotationLayerDeleteIntegrityError as ex:
return self.response_422(message=str(ex))
Expand Down Expand Up @@ -288,7 +288,7 @@ def put(self, pk: int) -> Response:
try:
new_model = UpdateAnnotationLayerCommand(g.user, pk, item).run()
return self.response(200, id=new_model.id, result=item)
except (AnnotationLayerNotFoundError) as ex:
except AnnotationLayerNotFoundError:
return self.response_404()
except AnnotationLayerInvalidError as ex:
return self.response_422(message=ex.normalized_messages())
Expand Down
2 changes: 1 addition & 1 deletion superset/annotation_layers/commands/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def run(self) -> Model:
return annotation_layer

def validate(self) -> None:
exceptions: List[ValidationError] = list()
exceptions: List[ValidationError] = []

name = self._properties.get("name", "")

Expand Down
2 changes: 1 addition & 1 deletion superset/annotation_layers/commands/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def run(self) -> Model:
return annotation_layer

def validate(self) -> None:
exceptions: List[ValidationError] = list()
exceptions: List[ValidationError] = []
name = self._properties.get("name", "")
self._model = AnnotationLayerDAO.find_by_id(self._model_id)

Expand Down
2 changes: 1 addition & 1 deletion superset/charts/commands/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def run(self) -> Model:
return chart

def validate(self) -> None:
exceptions = list()
exceptions = []
datasource_type = self._properties["datasource_type"]
datasource_id = self._properties["datasource_id"]
dashboard_ids = self._properties.get("dashboards", [])
Expand Down
2 changes: 1 addition & 1 deletion superset/charts/commands/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def run(self) -> Model:
return chart

def validate(self) -> None:
exceptions: List[ValidationError] = list()
exceptions: List[ValidationError] = []
dashboard_ids = self._properties.get("dashboards")
owner_ids: Optional[List[int]] = self._properties.get("owners")

Expand Down
2 changes: 1 addition & 1 deletion superset/commands/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def populate_owners(
:returns: Final list of owners
"""
owner_ids = owner_ids or []
owners = list()
owners = []
if not owner_ids and default_to_user:
return [user]
if user.id not in owner_ids and "admin" not in [
Expand Down
4 changes: 1 addition & 3 deletions superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -471,9 +471,7 @@ def data(self) -> Dict[str, Any]:
)


class SqlaTable(
Model, BaseDatasource
): # pylint: disable=too-many-instance-attributes,too-many-public-methods
class SqlaTable(Model, BaseDatasource): # pylint: disable=too-many-public-methods
"""An ORM object for SqlAlchemy table references"""

type = "table"
Expand Down
2 changes: 1 addition & 1 deletion superset/dashboards/commands/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def run(self) -> Model:
return dashboard

def validate(self) -> None:
exceptions: List[ValidationError] = list()
exceptions: List[ValidationError] = []
owner_ids: Optional[List[int]] = self._properties.get("owners")
role_ids: Optional[List[int]] = self._properties.get("roles")
slug: str = self._properties.get("slug", "")
Expand Down
2 changes: 1 addition & 1 deletion superset/databases/commands/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def run(self) -> Model:
return database

def validate(self) -> None:
exceptions: List[ValidationError] = list()
exceptions: List[ValidationError] = []
sqlalchemy_uri: Optional[str] = self._properties.get("sqlalchemy_uri")
database_name: Optional[str] = self._properties.get("database_name")
if not sqlalchemy_uri:
Expand Down
2 changes: 1 addition & 1 deletion superset/databases/commands/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def run(self) -> Model:
return database

def validate(self) -> None:
exceptions: List[ValidationError] = list()
exceptions: List[ValidationError] = []
# Validate/populate model exists
self._model = DatabaseDAO.find_by_id(self._model_id)
if not self._model:
Expand Down
2 changes: 1 addition & 1 deletion superset/datasets/commands/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def run(self) -> Model:
return dataset

def validate(self) -> None:
exceptions: List[ValidationError] = list()
exceptions: List[ValidationError] = []
database_id = self._properties["database"]
table_name = self._properties["table_name"]
schema = self._properties.get("schema", None)
Expand Down
2 changes: 1 addition & 1 deletion superset/datasets/commands/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def run(self) -> Model:
raise DatasetUpdateFailedError()

def validate(self) -> None:
exceptions: List[ValidationError] = list()
exceptions: List[ValidationError] = []
owner_ids: Optional[List[int]] = self._properties.get("owners")
# Validate/populate model exists
self._model = DatasetDAO.find_by_id(self._model_id)
Expand Down
2 changes: 1 addition & 1 deletion superset/datasets/dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ def update_metrics(
- If there are extra metrics on the metadata db that are not defined on the List
then we delete.
"""
new_metrics = list()
new_metrics = []
for metric in property_metrics:
metric_id = metric.get("id")
if metric.get("id"):
Expand Down
4 changes: 1 addition & 3 deletions superset/models/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,7 @@ def copy_dashboard(
)


class Dashboard( # pylint: disable=too-many-instance-attributes
Model, AuditMixinNullable, ImportExportMixin
):
class Dashboard(Model, AuditMixinNullable, ImportExportMixin):
"""The dashboard object!"""

__tablename__ = "dashboards"
Expand Down
2 changes: 1 addition & 1 deletion superset/reports/commands/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def run(self) -> Model:
return report_schedule

def validate(self) -> None:
exceptions: List[ValidationError] = list()
exceptions: List[ValidationError] = []
owner_ids: Optional[List[int]] = self._properties.get("owners")
name = self._properties.get("name", "")
report_type = self._properties.get("type")
Expand Down
2 changes: 1 addition & 1 deletion superset/reports/commands/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def run(self) -> Model:
return report_schedule

def validate(self) -> None:
exceptions: List[ValidationError] = list()
exceptions: List[ValidationError] = []
owner_ids: Optional[List[int]] = self._properties.get("owners")
report_type = self._properties.get("type", ReportScheduleType.ALERT)

Expand Down
8 changes: 3 additions & 5 deletions superset/tasks/async_queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def load_chart_data_into_cache(
raise exc
except Exception as exc:
# TODO: QueryContext should support SIP-40 style errors
error = exc.message if hasattr(exc, "message") else str(exc) # type: ignore # pylint: disable=no-member
error = exc.message if hasattr(exc, "message") else str(exc) # type: ignore
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@villebro as a side note I might do a pass to remain exc to ex for consistency throughout the code.

errors = [{"message": error}]
async_query_manager.update_job(
job_metadata, async_query_manager.STATUS_ERROR, errors=errors
Expand Down Expand Up @@ -122,11 +122,9 @@ def load_explore_json_into_cache( # pylint: disable=too-many-locals
raise ex
except Exception as exc:
if isinstance(exc, SupersetVizException):
errors = exc.errors # pylint: disable=no-member
errors = exc.errors
else:
error = (
exc.message if hasattr(exc, "message") else str(exc) # type: ignore # pylint: disable=no-member
)
error = exc.message if hasattr(exc, "message") else str(exc) # type: ignore
errors = [error]

async_query_manager.update_job(
Expand Down
4 changes: 2 additions & 2 deletions superset/utils/dict_import_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def export_schema_to_dict(back_references: bool) -> Dict[str, Any]:
clusters = [
DruidCluster.export_schema(recursive=True, include_parent_ref=back_references)
]
data = dict()
data = {}
if databases:
data[DATABASES_KEY] = databases
if clusters:
Expand Down Expand Up @@ -69,7 +69,7 @@ def export_to_dict(
for cluster in cls
]
logger.info("Exported %d %s", len(clusters), DRUID_CLUSTERS_KEY)
data = dict()
data = {}
if databases:
data[DATABASES_KEY] = databases
if clusters:
Expand Down
4 changes: 2 additions & 2 deletions superset/utils/sqllab_execution_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
SqlResults = Dict[str, Any]


@dataclass # pylint: disable=R0902
class SqlJsonExecutionContext:
@dataclass
class SqlJsonExecutionContext: # pylint: disable=too-many-instance-attributes
database_id: int
schema: str
sql: str
Expand Down
2 changes: 1 addition & 1 deletion superset/views/base_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def pre_load(self, data: Dict[Any, Any]) -> None:

@staticmethod
def set_owners(instance: Model, owners: List[int]) -> None:
owner_objs = list()
owner_objs = []
if g.user.get_id() not in owners:
owners.append(g.user.get_id())
for owner_id in owners:
Expand Down