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

Fix WorkersByJobId: Filtered Workers Not Empty Check #699

Merged
merged 2 commits into from
Jul 31, 2024

Conversation

kmg-stripe
Copy link
Collaborator

@kmg-stripe kmg-stripe commented Jul 30, 2024

Context

I forgot to update the previous PR with this fix. We have enough cruft lying around from all of the churn, that we hit cases where we get a NPE when we cannot parse the worker payload in getAllWorkersByJobId. This checks to ensure workers is not empty before referencing the first element of workers.

Checklist

  • ./gradlew build compiles code correctly
  • Added new tests where applicable
  • ./gradlew test passes all tests
  • Extended README or added javadocs where applicable

Copy link

github-actions bot commented Jul 30, 2024

Test Results

537 tests  +214   531 ✅ +214   7m 46s ⏱️ + 1m 4s
139 suites + 44     6 💤 ±  0 
139 files   + 44     0 ❌ ±  0 

Results for commit 43f1c11. ± Comparison against base commit ef3ad82.

♻️ This comment has been updated with latest results.

@kmg-stripe
Copy link
Collaborator Author

@hmitnflx - If you have a sec, could you have a look at this one? Should be quick.

@crioux-stripe
Copy link
Collaborator

@Andyz26 Hey! One more for our production readiness if you don't mind.

@kmg-stripe kmg-stripe merged commit a293400 into Netflix:master Jul 31, 2024
4 of 5 checks passed
@crioux-stripe
Copy link
Collaborator

crioux-stripe commented Jul 31, 2024

@Andyz26 Sorry about self-approving on this one. I wanted to make sure @kmg-stripe could hit our target of releasing today.

@Andyz26
Copy link
Collaborator

Andyz26 commented Aug 5, 2024

@crioux-stripe no worries. I am on vacation and I will check with @hmit on this.

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.

3 participants