Codingaditya17 commented on issue #67200:
URL: https://github.com/apache/airflow/issues/67200#issuecomment-4501179664

   Thanks for sharing the draft. I did a quick first pass and the overall 
direction looks good to me. The `AssetState(name=...)` / `AssetState(uri=...)` 
API feels cleaner than exposing separate name/uri getters directly.
   
   Since this is still a draft and CI is currently failing on static checks, 
docs spellcheck, and Task SDK tests, I would wait before doing a full review. 
One thing I noticed from the diff is that the `ToTriggerRunner` union around 
`MaskSecret` may need cleanup, since it looks like there may be duplicate 
`MaskSecret` / comma placement issues.
   
   Also, since the PR description mentions usage from both Trigger and Task, I 
think it would be good to add or confirm task-side coverage too, along with 
validation tests for both/neither `name` and `uri`.
   
   Happy to re-review after the next update.


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

Reply via email to