Skip to content

Light clean: add C# implementation, tests, and fix things!#8

Closed
nathanwl88 wants to merge 7 commits intomainfrom
dotnet-light-clean
Closed

Light clean: add C# implementation, tests, and fix things!#8
nathanwl88 wants to merge 7 commits intomainfrom
dotnet-light-clean

Conversation

@nathanwl88
Copy link
Copy Markdown
Contributor

@nathanwl88 nathanwl88 commented Mar 13, 2024

Added light clean for C# so it can be ran inside a C# web job.

@nathanwl88 nathanwl88 requested a review from JOT85 March 13, 2024 12:33
@JOT85 JOT85 changed the title Added c# light clean Light clean: add C# implementation, tests, and fix things! Mar 15, 2024
Copy link
Copy Markdown
Member

@JOT85 JOT85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR has become more of a "add tests for weird cleaning cases" then "make sure everything works in weird cases"... I think we're mostly good. I still need to add tests for the Node.js implementation of light clean, and the dotnet implementation is failing the tests, but the Python one seems to work!

See my comment on line 218-220 above.

Python + dotnet only

Python cleaner

./run-test.sh -t python_jobs,dotnet_jobs

Passing

Dotnet cleaner

./run-test.sh -t python_jobs,dotnet_jobs -c ./dotnet-cleaner/run.sh

Failing

Full tests

Python cleaner

./run-test.sh -t go_jobs,python_jobs,rust_jobs,node_jobs,dotnet_jobs -c ./dotnet-cleaner/run.sh

Passing

Dotnet cleaner

./run-test.sh -t go_jobs,python_jobs,rust_jobs,node_jobs,dotnet_jobs -c ./dotnet-cleaner/run.sh

Failing

Comment on lines +218 to +220
//FreeRedis LPos always returns a long
//need to confirm if -1 is returned in place of NIL
if (DataExists(db, item_id) && db.LPos(MainQueueKey, item_id) < 0 && db.LPos(ProcessingKey, item_id) < 0)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect LPos might return 0 if the response is nil... Which makes it impossible to use this method to determine if the item is in the queue...

The tests are failing (for a simple case that only requires Python and dotnet, and uses the dotnet cleaning, from within the tests directory, run ./run-test.sh -t python_jobs,dotnet_jobs -c ./dotnet-cleaner/run.sh) and I think this is the reason it fails 😞

@JOT85
Copy link
Copy Markdown
Member

JOT85 commented Aug 27, 2024

Superseded by #11

@JOT85 JOT85 closed this Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants