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

Replace panic with error message when not declaring a source node in .yml config #1695

Merged
merged 6 commits into from
Jul 30, 2024

Conversation

AdamHaffar
Copy link
Contributor

@AdamHaffar AdamHaffar commented Jul 4, 2024

Description

Fixes an issue where the check for len(sourceNodes) in buildNodes() was never true due to a DLQHandlerNode always being appended to nodes resulting in a minimum length of 1 for sourceNodes.

Moves append for DLQHandlerNode to only occur if there are source nodes available. This in turn automatically improves the error message instead of a panic.

Fixes #1629

Quick checks:

  • I have followed the Code Guidelines.
  • There is no other pull request for the same update/change.
  • I have written unit tests.
  • I have made sure that the PR is of reasonable size and can be easily reviewed.

@AdamHaffar AdamHaffar requested a review from a team as a code owner July 4, 2024 16:31
@AdamHaffar AdamHaffar changed the title Prevent panic from not declaring a source node in .yml config Replace panic with error message when not declaring a source node in .yml config Jul 4, 2024
@hariso
Copy link
Contributor

hariso commented Jul 8, 2024

@AdamHaffar Thanks for this PR! I think we should handle the case with 0 destination nodes too. I know the original issue is about source nodes, but I believe it's a similar problem for destinations as well.

@lovromazgon lovromazgon removed their assignment Jul 18, 2024
@lovromazgon lovromazgon requested a review from hariso July 30, 2024 17:22
@hariso hariso enabled auto-merge (squash) July 30, 2024 17:24
@hariso hariso merged commit 7339241 into main Jul 30, 2024
3 checks passed
@hariso hariso deleted the adam/better-error-message branch July 30, 2024 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants