You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "pierrejeambrun (via GitHub)" <gi...@apache.org> on 2024/10/08 16:07:15 UTC
[PR] AIP-84 Get Variable [airflow]
pierrejeambrun opened a new pull request, #42834:
URL: https://github.com/apache/airflow/pull/42834
Related to: https://github.com/apache/airflow/issues/42370
Add delete_variable endpoint.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Variable [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #42834:
URL: https://github.com/apache/airflow/pull/42834#issuecomment-2402128272
> I know this is just copying the existing REST API. But I don't see any checks to try to hide sensitive variable values. Do we want to add that in?
Good idea. I'm trying to see if I can incorporate that real quick, don't know why my test is failing at the moment.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Variable [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #42834:
URL: https://github.com/apache/airflow/pull/42834#discussion_r1792350882
##########
airflow/api_fastapi/openapi/v1-generated.yaml:
##########
@@ -1037,3 +1081,25 @@ components:
- msg
- type
title: ValidationError
+ VariableResponse:
+ properties:
+ key:
Review Comment:
Why do we actually re turn the key in the response if it was in the request... sounds use-less... but anyway is also in previous API :-D
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Variable [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #42834:
URL: https://github.com/apache/airflow/pull/42834#issuecomment-2401781527
I know this is just copying the existing REST API. But I don't see any checks to try to hide sensitive variable values. Do we want to add that in?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Variable [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun closed pull request #42834: AIP-84 Get Variable
URL: https://github.com/apache/airflow/pull/42834
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Variable [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #42834:
URL: https://github.com/apache/airflow/pull/42834
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Variable [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #42834:
URL: https://github.com/apache/airflow/pull/42834#issuecomment-2402248275
Pushed. Test cases were updated.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Variable [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #42834:
URL: https://github.com/apache/airflow/pull/42834#discussion_r1793064275
##########
airflow/api_fastapi/openapi/v1-generated.yaml:
##########
@@ -1037,3 +1081,25 @@ components:
- msg
- type
title: ValidationError
+ VariableResponse:
+ properties:
+ key:
Review Comment:
Yes because it would mean adapting the response to each endpoint and request depending on the query params and path param to substract them from the response. That would be a lot of extra work, and the returned schema would dynamically change, that would be hard for the client to know what is the actual 'body' schema for that specific request.
So usually we don't bother with this even if the data is indeed duplicated by returning the full entity.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Variable [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #42834:
URL: https://github.com/apache/airflow/pull/42834#discussion_r1793064275
##########
airflow/api_fastapi/openapi/v1-generated.yaml:
##########
@@ -1037,3 +1081,25 @@ components:
- msg
- type
title: ValidationError
+ VariableResponse:
+ properties:
+ key:
Review Comment:
Yes because it would mean adapting the response to each endpoint and request depending on the query params and path param to substract them from the response. That would be a lot of extra work, and the returned schema would dynamically change, that would be hard for the client to know what is the actual 'body' schema for that specific request.
So usually we don't bother with this even if the data is indeed duplicated.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org