potiuk commented on code in PR #35867:
URL: https://github.com/apache/airflow/pull/35867#discussion_r1408952744
##########
airflow/serialization/serde.py:
##########
@@ -83,6 +91,81 @@ def decode(d: dict[str, Any]) -> tuple[str, int, Any]:
return classname, version, data
+def encrypt(value: P) -> str:
Review Comment:
And just to add a bit of more context here... I woudl like to become serde
expert.
We seem to have a bit shortage of these - I think. Very recently it turned
out that "likely" we've done something wrong when reviewing providers code that
was supposed to use serde even if we did not understand it (I still don't see
why we would use it even after carefully reading
https://github.com/apache/airflow/pull/35885 - which is BTW. an excellent
description of what serde does and how it can be used and we should get it in
soon - with the caveat of not being part of airflow "user" documentatiom, more
"collaborator/contributor" as I commented there.
So my goal is to help to make serde first-class citizen and really used -
but in order to do that I need to understand
it - i am missing a lot of "whys" - i.e. context. alternatives, why we
selected certain ways of doing it and whether we fully understand consequences
of what we are doing - especially that what was simple "Seriialize object to
bytes" now starts getting security "properties" and evidently we want it to be
used everywhere.
Again - I am not saying it's wrong, I am saying - I do not understand it ,
and I would like to - especially if I am to review the code.
We can do it in a number of ways. One of the ways is to discuss every single
PR and get back-forth discussions like this where I might be asking possibly
stupid questions. Another way (and I precticed it - for example - in Breeze/CI
where I brought more people in and what we have is set of Architecture Decision
Records https://github.com/apache/airflow/tree/main/dev/breeze/doc/adr - where
I can easily send anyone to if they want to know answers why we have made
certain decisions - with "context", "decision" and "consequences" put together.
Of course it is not read on a regular basis, but at least anyone can refer to
it and find out why some important decisions have been mande and - more
importantly - we can question our past individual decision and modify them
- by knowing why we made the decisions, what were the decisions and
consequences. Very useful especially when there are new people and we want them
to be able to make reviews or potentially revert/question/change decisions made
in the past because we have new circumstances.
I am not saying we need exactly this format, but I think IF we want serde
really used, we need to understand some of the decisions made there -
especially if we are going to add security to the mix and likely in a form that
is approachable by anyone (like me) who had not participated in past decisions
and discussions.
I want to understand, that's 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
