aminghadersohi commented on code in PR #44695:
URL: https://github.com/apache/superset/pull/44695#discussion_r4129930167
##########
superset/db_engine_specs/gsheets.py:
##########
@@ -64,6 +66,28 @@
ma_plugin = MarshmallowPlugin()
+def to_json_value(value: Any) -> Any:
+ """
+ Convert a dataframe cell into a JSON value the Sheets API parses back.
+
+ Dates and timestamps become ISO strings (``USER_ENTERED`` input parses them
+ as dates), and numpy scalars become Python scalars.
+ """
+ if value is None:
+ return None
+ if isinstance(value, pd.Timestamp):
+ value = value.to_pydatetime()
+ if isinstance(value, datetime):
+ return value.isoformat(sep=" ")
+ if isinstance(value, (date, time)):
+ return value.isoformat()
+ if isinstance(value, Decimal):
+ return str(value)
+ if isinstance(value, np.generic):
+ return value.item()
Review Comment:
Verified already fixed in 2f9e93110f56a3e95b5d400c723a819b1b7f5496:
_to_python_value normalizes timestamps and durations, and to_json_value
serializes their Python equivalents; the parameterized numpy date/duration
tests pass at the updated head.
##########
superset/db_engine_specs/gsheets.py:
##########
@@ -64,6 +66,43 @@
ma_plugin = MarshmallowPlugin()
+def _to_python_value(value: Any) -> Any:
+ """
+ Convert numpy and pandas scalars into their Python equivalents.
+ """
+ if isinstance(value, np.datetime64):
+ # ``.item()`` returns an int for nanosecond precision; go through
pandas.
+ value = pd.Timestamp(value)
+ elif isinstance(value, np.generic):
+ # e.g. ``np.timedelta64.item()`` returns a ``timedelta``.
+ value = value.item()
Review Comment:
Reverified already fixed in 6211f59beff6d8d7df526226a7015a38912eee29:
np.timedelta64 is handled before np.generic via pd.Timedelta, and the 5 ns,
90-second ns and NaT regression cases all pass.
##########
superset/db_engine_specs/gsheets.py:
##########
@@ -411,11 +436,13 @@ def validate_parameters(
)
return errors
- # We need a subject in case domain wide delegation is set, otherwise
the
- # check will fail. This means that the admin will be able to add sheets
- # that only they have access, even if later users are not able to
access
- # them.
- subject = g.user.email if g.user else None
+ # Impersonate the admin only when the database impersonates users, as
+ # queries do (see ``impersonate_user``). With domain wide delegation
this
+ # means that the admin will be able to add sheets that only they have
+ # access to; without delegation a subject makes every check fail.
+ subject = (
+ g.user.email if g.user and properties.get("impersonate_user") else
None
+ )
Review Comment:
Disputed: at 6211f59beff6d8d7df526226a7015a38912eee29,
superset/models/core.py:524-538 (Database.get_effective_user) prefers
get_username() over the URL username, and
superset/db_engine_specs/gsheets.py:286-299 (impersonate_user) looks up that
user’s email, so an authenticated validation request uses the same principal
rather than the URL user.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
