Ankurdeewan commented on code in PR #56324:
URL: https://github.com/apache/airflow/pull/56324#discussion_r2578041844


##########
tests/system/providers/google/common/example_quota_project_system.py:
##########
@@ -0,0 +1,112 @@
+# Licensed to the Apache Software Foundation (ASF) under one

Review Comment:
   Hi @VladaZakharova,
   
   Yeah, after looking at this again, you were right earlier. I only validated 
this through the unit tests, not a full end-to-end run, and that’s also why the 
system test blew up. The unit tests already cover all the important parts of 
the quota_project behavior, so adding a system test heree doesn’t really add 
anything.
   
   For context, the unit tests check everything that our code is actually 
responsible for. They verify the parameter gets stored correctly, that 
quota_project can be pulled from connection extras, that the parameter 
overrides the connection value, and that invalid values raise the right errors. 
They also check the error path when the credentials object raises while trying 
to apply the quota project. I stepped through the hook code locally too just to 
make sure the value flows properly. The part where the hook decides which ID to 
use and then validates it and applies it to the credentials object is working 
the way it should. The tests hit all the branches that matter in that area.
   
   To be honest, I didn’t run a real GCP integration test with quota 
attribution because that would require a whole GCP setup with the right IAM 
roles, two projects, billing enabled and so on. The unit tests already confirm 
the hook passes the quota project to the credentials in the right order and 
handles failures correctly. Anything beyond that is on Google’s side, not ours.
   
   Given all that, it makes more sense to just remove the system test. The unit 
tests already prove the code works as intended, and the system test will keep 
failing in CI without a full ADC setup.
   
   If you're ok with that direction, I can update the PR.



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