[GitHub] [superset] betodealmeida commented on a change in pull request #16160: feat: change query predicate to text

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

betodealmeida commented on a change in pull request #16160:
URL: https://github.com/apache/superset/pull/16160#discussion_r686322457



##########
File path: .pylintrc
##########
@@ -251,7 +251,7 @@ expected-line-ending-format=
 # Logging modules to check that the string format arguments are in logging
 # function parameter format
 logging-modules=logging
-
+disable=logging-fstring-interpolation

Review comment:
       ```suggestion
   ```
   
   Using f-strings with logging is not considered safe because if the string interpolation fails nothing gets logged. if you use the old-school string interpolation then the message always get logged, even if the impersonation fails.

##########
File path: superset/migrations/versions/07071313dd52_change_fetch_values_predicate_to_text.py
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""change_fetch_values_predicate_to_text
+
+Revision ID: 07071313dd52
+Revises: 6d20ba9ecb33
+Create Date: 2021-08-09 17:32:56.204184
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "07071313dd52"
+down_revision = "6d20ba9ecb33"
+
+import logging
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy import and_, func, or_
+from sqlalchemy.dialects import postgresql
+from sqlalchemy.sql.schema import Table
+
+from superset import db
+from superset.connectors.sqla.models import SqlaTable
+
+
+def upgrade():
+    with op.batch_alter_table("tables") as batch_op:
+        batch_op.alter_column(
+            "fetch_values_predicate",
+            existing_type=sa.String(length=1000),
+            type_=sa.Text(),
+            existing_nullable=True,
+        )
+
+
+def remove_value_if_too_long():
+    bind = op.get_bind()
+    session = db.Session(bind=bind)
+
+    # it will be easier for users to notice that their field has been deleted rather than truncated
+    # so just remove it if it won't fit back into the 1000 string length column
+    try:
+        rows = (
+            session.query(SqlaTable)
+            .filter(func.length(SqlaTable.fetch_values_predicate) > 1000)
+            .all()
+        )
+
+        for row in rows:
+            row.fetch_values_predicate = None
+
+        logging.info(f"{len(rows)} values deleted")

Review comment:
       ```suggestion
           logging.info("%d values deleted", len(rows))
   ```

##########
File path: .pylintrc
##########
@@ -251,7 +251,7 @@ expected-line-ending-format=
 # Logging modules to check that the string format arguments are in logging
 # function parameter format
 logging-modules=logging
-
+disable=logging-fstring-interpolation

Review comment:
       ```suggestion
   ```
   
   Using f-strings with logging is not considered safe because if the string interpolation fails nothing gets logged. if you use the old-school string interpolation then the message always get logged, even if the interpolation fails.




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