L1nq0 commented on PR #9094: URL: https://github.com/apache/storm/pull/9094#issuecomment-6009257029
@reiabreu Thanks for the review. Commit c3d3747, on top of the rebased branch, addresses the points. The redundant TupleDeserializationException entry is dropped from the tolerated set; the comment above the list stays. This also covers the second point from GGraziadei's review. The two-argument constructor is removed. It was added at rzo1's request in the first review round; if you would rather keep it, say so and it goes back in. IdDictionary.getStreamName now returns null for a component that is not in the dictionary, so both unresolved-routing cases reach the typed exception. A test covers the unknown-component lookup. Previously the inner map lookup threw a NullPointerException, which is not in the tolerated set and stayed fatal even in default mode. On the design question: yes, the type is a forward step for #9077. In default mode it is tolerated through its IllegalArgumentException supertype exactly like the bare IAE it replaces; the behavioral change in this PR comes from the new throw for unknown stream ids. Branching on the type is the follow-up work. The unknown-stream test now derives the id one past the component's declared stream count instead of the hardcoded 3. The points GGraziadei raised are answered on their threads. The strict-mode documentation now separates the two outcomes, and the behavior question is filed as #9157. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
