ahilashsasidharan commented on PR #63994:
URL: https://github.com/apache/airflow/pull/63994#issuecomment-4193791941
> Question for @o-nikolas and others: I pushed an implementation that moves
the check to `airflow-core/src/airflow/api_fastapi/core_api/routes/public/`
instead of in `airflow-core/src/airflow/api_fastapi/core_api/datamodels/`.
After some initial testing, both implementation work, so I wanted some feedback
on which one is preferred.
>
> Looking at the code I feel under` routes/public` might be more appropriate
for this type of check, but it requires more code spread across each file
including additional code to handle checking bulk requests. On the other hand
the `datamodel `approach is simpler and reviewers were previously fine with it.
>
> Happy to go with whichever is preferred.
One thing to add for this is that having these checks gives us more control
over how errors are sent for Bulk endpoints. Right now the implemenation for
errors looks like this:
`Error: 400 {"detail":{"message":"team_name cannot be set when multi_team
mode is disabled. Please contact your
administrator.","invalid_variable_keys":["team_a_api_key","team_a_bucket","team_b_api_key"]}`
This would be the error would like with the check being in datamodels with a
model_validator (with more failing actions/entities):
`Error: 422
{"detail":[{"type":"value_error","loc":["body","actions",1,"create","entities",0],"msg":"Value
error, team_name cannot be set when multi_team mode is
disabled","input":{"key":"team_a_api_key","value":"abc123","description":"API
key for team
A","team_name":"team_a"},"ctx":{"error":{}}},{"type":"value_error","loc":["body","actions",1,"create","entities",1],"msg":"Value
error, team_name cannot be set when multi_team mode is
disabled","input":{"key":"team_a_bucket","value":"s3://team-a-bucket","description":"S3
bucket for team
A","team_name":"team_a"},"ctx":{"error":{}}},{"type":"value_error","loc":["body","actions",1,"create","entities",2],"msg":"Value
error, team_name cannot be set when multi_team mode is
disabled","input":{"key":"team_b_api_key","value":"xyz789","description":"API
key for team
B","team_name":"team_b"},"ctx":{"error":{}}},{"type":"value_error","loc":["body","actions",1,"create","entities",0],"msg":"Value
error, team_name cannot be set when multi_team mod
e is
disabled","input":{"key":"team_a_api_key","value":"abc123","description":"API
key for team
A","team_name":"team_a"},"ctx":{"error":{}}},{"type":"value_error","loc":["body","actions",1,"create","entities",1],"msg":"Value
error, team_name cannot be set when multi_team mode is
disabled","input":{"key":"team_a_bucket","value":"s3://team-a-bucket","description":"S3
bucket for team
A","team_name":"team_a"},"ctx":{"error":{}}},{"type":"value_error","loc":["body","actions",1,"create","entities",2],"msg":"Value
error, team_name cannot be set when multi_team mode is
disabled","input":{"key":"team_b_api_key","value":"xyz789","description":"API
key for team B","team_name":"team_b"},"ctx":{"error":{}}}]}`
--
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]
