Closed Ravisaketi closed 2 years ago
CLA Assistant Lite bot All contributors have signed the CLA ✍️
Hi, @c0c0n3 @chicco785 when i run the tests locally all tests were passed but in the CircleCi the test_others
tests are failing. Sometimes after running so many times, it will pass automatically.
@Necravisaketi see my comment on #661. It seems bizarre, but it looks like adding log statements to WQ causes that error? Can't believe it, but can we try getting to the bottom of this?
@Necravisaketi actually I'm wrong about test_others
failures being related to this PR. If you look at #660 you'll see there's no code changes in there, but test_others
still failed. I've opened a separate issue about it: #666. Meanwhile we can ignore the failing test here, rebase on master after merging #661, then merge this PR. Thanks alot!
@Necravisaketi can you please run the linter on locally on your branch, see what files it changes and figure out what goes wrong? thanks alot, much appreciated!
@Necravisaketi it looks like this PR is good to go? There's a couple of issues but I don't think they've got anything to do w/ your code:
Should we merge or would you still like to add more code?
Hi, @c0c0n3 I have done from my side for this issue and there are no conflicts with these changes so we need to merge this pr.
good stuff, thanks alot @Necravisaketi!
Proposed changes
Added logs in src/wq/core/task.py. fix for issue #662
Types of changes
What types of changes does your code introduce to the project? Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...