Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RQ: simplify logging of jobs #4311

Merged
merged 1 commit into from
Nov 27, 2019
Merged

RQ: simplify logging of jobs #4311

merged 1 commit into from
Nov 27, 2019

Conversation

rauchy
Copy link
Contributor

@rauchy rauchy commented Nov 26, 2019

RQ logs the arguments the task was called with. This creates too verbose logs and might leak information where it not supposed to go (specially in the case of execute_query).

We should switch it log only the job name.

@rauchy rauchy requested a review from arikfr November 26, 2019 22:33
@rauchy rauchy moved this from To do to In progress in Switch from Celery to RQ Nov 26, 2019
@rauchy rauchy moved this from In progress to Review in progress in Switch from Celery to RQ Nov 26, 2019
@arikfr arikfr merged commit 80f3ec1 into master Nov 27, 2019
Switch from Celery to RQ automation moved this from Review in progress to Done Nov 27, 2019
@arikfr arikfr deleted the avoid-logging-job-parameters branch November 27, 2019 07:33
@arikfr
Copy link
Member Author

arikfr commented Nov 27, 2019

For some reason GitHub kept requesting me for a comment when I tried to leave an approval review, so I just merged...

@rauchy
Copy link
Contributor

rauchy commented Nov 27, 2019

Here are a few suggestions for next time:

Oh boy, terrific job!
This PR has taught me so much, thank you!
At first I thought this was an impossible task, but you proved me wrong.
This is code craftsmanship at it's finest.
You make me want to be a better programmer.
Holy shit, you nailed it!

I can go on.

@arikfr
Copy link
Member Author

arikfr commented Nov 27, 2019

@rauchy thanks! This is very helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants