Codecov Report

Base: 87.00% // Head: 87.01% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (700f5ba) compared to base (cb83eaf).
Patch coverage: 97.22% of modified lines in pull request are covered.

@@            Coverage Diff             @@
##             main    #8045      +/-   ##
==========================================
+ Coverage   87.00%   87.01%   +0.01%     
==========================================
  Files         148      148              
  Lines       18486    18440      -46     
  Branches     2520     2514       -6     
==========================================
- Hits        16083    16045      -38     
+ Misses       2123     2117       -6     
+ Partials      280      278       -2     

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

splitting to smaller chunks only or more improvement on the existing implementation?

splitting to smaller chunks only or more improvement on the existing implementation?

Basically, the stamping mechanism is our main blocker for Celery 5.3 release.
At the moment, Celery 5 (main) has bugs in an environment that includes both Celery 4 & 5 workers, due to the stamping feature, And the stamping feature itself has even more bugs by itself (Celery 5 vs Celery 5 worker environment) - mostly edge cases, but still critical edge cases.

So we can't release Celery 5 as it will break support in Celery 4 hybrid worker environment and that's a critical release blocker (imagine the process of migrating from 4 -> 5 and not having a hybrid environment working, it will be a nightmare to upgrade).

The stamping mechanism feature was developed before I was involved with Celery (at all tbh), so I am trying to complete the feature (reverse engineering the design, etc.) to get it working so we can advance into Celery 5.3 release.

The general idea of this PR then is to refactor the feature's implementation, reduce its complexity, complete testing, etc.

P.S
After I get the main to a stable version regarding bugs/features, I will push with the new pytest-celery plugin to allow implementing more advanced tests before Celery 5.3 release (e.g, backend/broker D/C tests, multiple prefork workers automatic tests, etc.).
This will create a very stable version (5.3) which we want to have, to gain trust in 5.3 and allow real-production environments the safety to upgrade.

@auvipy

thank you for your in detail response

@thedrow I've replied to all of your comments.
All of the required fixes are pushed.
Please review again and notice:

1. The integration tests are basically untouched from main at this point, only moved to a different file so far into the PR.
2. Sphinx documentation will be the last step, to make sure we have a stable version first.

@auvipy @thedrow
Do you have any idea why does the code coverage is failing?

Failing after 1s — 86.99% (-0.01%) compared to cb83eaf

@auvipy @thedrow Do you have any idea why does the code coverage is failing?

Failing after 1s — 86.99% (-0.01%) compared to cb83eaf

that's mostly random codcov issue I guess

@auvipy @thedrow Do you have any idea why does the code coverage is failing?

Failing after 1s — 86.99% (-0.01%) compared to cb83eaf

that's mostly random codcov issue I guess

I think I found the reason it failed. Very interesting!
Fixed in this commit: db7936b

The code that I removed in this commit was code that indeed ran, but the condition if stamp in self.options and stamp not in link.options: was always false after all of the fixes and changes of this PR.
So technically, the body of the condition was never accessed, thus causing a code coverage error.

Now the interesting part - investigating this, I discovered that the original code (the one I just removed in db7936b) was actually a patch to a bug in the stamping mechanism, which while the bug existed this code was accessed so it did not have any code cov errors when it was first introduced. As this PR fixes the stamping mechanism, the patch fixes an impossible edge case now (as it is naturally handled now, without need for edge cases patches), so the workaround wasn't needed anymore, making the code coverage error in the PR actually very useful in finding out useless code that can be removed safely.

It was damn hard to understand but finally I can get my 100% passing CI, I hate seeing errors! 😁

@auvipy @thedrow
I think this makes a very important lesson - we shouldn't ignore very easily CI errors, even if it's "just code coverage".

Thank you!! 🙏 🙏