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

Bugfix/567 heatmap race condition #568

Merged
merged 3 commits into from
May 17, 2021

Conversation

davidzwa
Copy link
Contributor

@davidzwa davidzwa commented May 12, 2021

  • Add heatmap tests + repro
  • Add farm info service
  • Fix startup index/app-core race condition OR patch the heatmap on the spot
  • Changelog

Unrelated but small changes:

  • Generic info to debug log changes
  • undefined to "undefined" change (although not the cause of problems thusfar)
  • Revert filamentManager renaming filamentManagerEnabled to avoid future headaches (either rename it all, or leave it untouched)

@davidzwa davidzwa self-assigned this May 12, 2021
]
}])

expect(PrinterClean.initFarmInformation()).rejects.toThrow("Cannot read property 'x' of undefined");
Copy link
Contributor Author

@davidzwa davidzwa May 12, 2021

Choose a reason for hiding this comment

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

Our undefined bug, repro'd here. If the bug is prevented, this test will start to fail.
If the race-condition is fixed, this test will still see exceptions thrown.

Choice: patch the database object on the spot, or fix the race condition

Copy link
Member

Choose a reason for hiding this comment

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

We could define the heat map in the database no? We'd only have to check for the day in the code if I remember rightly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my change below, I think you're right but I saw incorrect duplication so maybe we can prevent this bug differently.

@davidzwa davidzwa marked this pull request as ready for review May 12, 2021 18:05
@davidzwa davidzwa force-pushed the bugfix/567-heatmap-race-condition branch 4 times, most recently from a2dcb91 to 57383de Compare May 16, 2021 18:28
@NotExpectedYet
Copy link
Member

I'll get this checked tomorrow and go through the file/job clean one once merged.

@davidzwa davidzwa force-pushed the bugfix/567-heatmap-race-condition branch from 07b3811 to 6a5e299 Compare May 17, 2021 08:10
Copy link
Member

@NotExpectedYet NotExpectedYet left a comment

Choose a reason for hiding this comment

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

Tested and all seems working from this end.

  • Existing Mongo Local on PC issue was discovered: Result - Original database recovered.
  • New Mongo Local on PC issue was discovered: Result - New database generated
  • Existing mongo db on Atlas: Result - Original database recovered
  • Remote mongo db on Atlas: Result - New Database Generated

Good spot!

@davidzwa davidzwa merged commit ef31981 into development May 17, 2021
@davidzwa davidzwa deleted the bugfix/567-heatmap-race-condition branch May 17, 2021 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

PrinterClean x of undefined when you start a new database, don't add printers and restart
2 participants