You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/12/23 18:50:04 UTC
[GitHub] [airflow] jsnb-devoted opened a new pull request, #28563: Change trigger name so kwargs arent logged
jsnb-devoted opened a new pull request, #28563:
URL: https://github.com/apache/airflow/pull/28563
I'm using the triggerer to make RPC calls and I'm passing in a bearer token as part of the trigger's kwargs. Because he name includes the `repr` of the whole instance it is logging the bearer token in the triggerer logs. This is as much a question as it is a suggestion. Could we possibly add the kwargs as another field in the dict and then only log at the DEBUG level?
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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
[GitHub] [airflow] jsnb-devoted commented on pull request #28563: Change trigger name so kwargs arent logged
Posted by GitBox <gi...@apache.org>.
jsnb-devoted commented on PR #28563:
URL: https://github.com/apache/airflow/pull/28563#issuecomment-1366059570
> > I'm passing in a bearer token as part of the trigger's kwargs
>
> That sounds like a bad idea, why do you need to do that?
@kaxil / @dstandish I'm doing this so that I don't have to keep authenticating with the API every time the trigger runs. I'm totally open to this being a bad idea but I was just trying to get an MVP of this working. Is the best practice here to have to authenticate every time the trigger runs?
Seemed like the logging of all the kwargs was more incidental than anything so I thought I would propose the change. If logging all of the kwargs was intended then no sweat if we want to close this.
--
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
[GitHub] [airflow] dstandish commented on pull request #28563: Change trigger name so kwargs arent logged
Posted by GitBox <gi...@apache.org>.
dstandish commented on PR #28563:
URL: https://github.com/apache/airflow/pull/28563#issuecomment-1364458694
BUT... i believe this will be stored in plain text in the db when trigger created / while trigger is running
--
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
[GitHub] [airflow] dstandish commented on pull request #28563: Change trigger name so kwargs arent logged
Posted by GitBox <gi...@apache.org>.
dstandish commented on PR #28563:
URL: https://github.com/apache/airflow/pull/28563#issuecomment-1364458436
ok @jsnb-devoted i think you may be able to make it so that your token will always be masked
see what's done here
https://github.com/apache/airflow/blob/2a34dc9e8470285b0ed2db71109ef4265e29688b/airflow/providers/slack/operators/slack.py#L55-L56
--
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
[GitHub] [airflow] kaxil commented on pull request #28563: Change trigger name so kwargs arent logged
Posted by GitBox <gi...@apache.org>.
kaxil commented on PR #28563:
URL: https://github.com/apache/airflow/pull/28563#issuecomment-1364394896
> I'm passing in a bearer token as part of the trigger's kwargs
That sounds like a bad idea, why do you do that?
--
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
[GitHub] [airflow] dstandish commented on pull request #28563: Change trigger name so kwargs arent logged
Posted by GitBox <gi...@apache.org>.
dstandish commented on PR #28563:
URL: https://github.com/apache/airflow/pull/28563#issuecomment-1364457263
hi @jsnb-devoted it would be helpful if you add a little more detail.
* example of what it looks like now (i'm sure none of us know off top of head) and what it would look like with your change
* just tad more detail about what change you have in mind. it was not quite clear to me.
thanks
--
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
[GitHub] [airflow] github-actions[bot] commented on pull request #28563: Change trigger name so kwargs arent logged
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #28563:
URL: https://github.com/apache/airflow/pull/28563#issuecomment-1426514763
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
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
[GitHub] [airflow] dstandish commented on pull request #28563: Change trigger name so kwargs arent logged
Posted by GitBox <gi...@apache.org>.
dstandish commented on PR #28563:
URL: https://github.com/apache/airflow/pull/28563#issuecomment-1364458418
ok @jsnb-devoted i think you may be able to make it so that your token will always be masked
see what's done here
https://github.com/apache/airflow/blob/2a34dc9e8470285b0ed2db71109ef4265e29688b/airflow/providers/slack/operators/slack.py#L55-L56
--
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
[GitHub] [airflow] dstandish commented on pull request #28563: Change trigger name so kwargs arent logged
Posted by GitBox <gi...@apache.org>.
dstandish commented on PR #28563:
URL: https://github.com/apache/airflow/pull/28563#issuecomment-1364457454
oh whoops ... i read this on phone initially ... did not realize it was PR :)
so, no need to add change -- that's in the code. but, before / after would be good.
--
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
[GitHub] [airflow] jsnb-devoted commented on pull request #28563: Change trigger name so kwargs arent logged
Posted by GitBox <gi...@apache.org>.
jsnb-devoted commented on PR #28563:
URL: https://github.com/apache/airflow/pull/28563#issuecomment-1366101427
@kaxil -- I misunderstood how the trigger classes were being instantiated. I've moved the code that authenticates with the API into the `__init__` function so that it runs the one time once it is instantiated. This fixed the bearer token logging issue.
I could still see not wanting to log the entire signature of the function in the triggerer but I'll leave that for you all to decide. Let me know or feel free to close the PR.
Thanks for the help!
--
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
[GitHub] [airflow] jsnb-devoted closed pull request #28563: Change trigger name so kwargs arent logged
Posted by "jsnb-devoted (via GitHub)" <gi...@apache.org>.
jsnb-devoted closed pull request #28563: Change trigger name so kwargs arent logged
URL: https://github.com/apache/airflow/pull/28563
--
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