john-bodley commented on code in PR #24301:
URL: https://github.com/apache/superset/pull/24301#discussion_r1232578110
##########
superset/queries/saved_queries/dao.py:
##########
@@ -44,3 +47,20 @@ def bulk_delete(models: Optional[list[SavedQuery]], commit:
bool = True) -> None
except SQLAlchemyError as ex:
db.session.rollback()
raise DAODeleteFailedError() from ex
+
+ @classmethod
+ def get_by_id(cls, _id: str) -> Optional[SavedQuery]:
Review Comment:
I agree with @michael-s-molina's comment regarding making this a SIP. I
understand that this feels somewhat like a blocker to what feels like a
somewhat benign fix, but I think it's important that we flush out (and
detangle) the key concepts that Michael and myself raised to set the right
precedence, as opposed to a bandaid solution.
Historically UUIDs were obfuscated and solely used to make charts,
dashboards, etc. globally unique across shared Superset instances, however
their use within the API is more of an organic phenomenon and thus I think
there's merit in pausing, taking a step back, and evaluating how we think about
UUIDs, shareable links, etc. in a more holistic manner.
I agree that finding a matching UUID is near impossible, but using UUIDs for
security (via cryptography) isn't valid. Note this is in no way a criticism of
your work, but more with the state of the Superset code base. At Airbnb we
support thousands of weekly active users and are constantly fixing issues
related to problematic features and/or inconsistent/non-coherent/complex
functionality (you can see the [vast
majority](https://github.com/apache/superset/pulls/john-bodley) of my commits
to this repo are actually fixes and/or mechanism to improve quality).
--
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]
