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

Issue1023 take2 topicCopy fix #1191

Open
wants to merge 4 commits into
base: development
Choose a base branch
from
Open

Conversation

petersilva
Copy link
Contributor

close #1023

  • um... again... topicCopy should work now.

Copy link

github-actions bot commented Aug 27, 2024

Test Results

239 tests   234 ✅  1m 36s ⏱️
  1 suites    1 💤
  1 files      4 ❌

For more details on these failures, see this check.

Results for commit 8f26778.

♻️ This comment has been updated with latest results.

@petersilva petersilva changed the title Issue1023 take2 Issue1023 take2 topicCopy fix Aug 27, 2024
@petersilva petersilva marked this pull request as ready for review August 27, 2024 16:02
@reidsunderland
Copy link
Member

I will do a bit more testing tomorrow.

My initial quick test seems like this is always copying the source message's topic, even with topicCopy False. But I could have done something wrong in my test too.

@petersilva
Copy link
Contributor Author

OK. I think with this last patch it does what people want.

  • if the topicCopy option is set, the inbound topic overrides and is copied to the published message.
  • if topicCopy is not set, then it tries to build a normal 'subtopic' ... if it can't it doesn't ... the message just doesn't get a topic or a subtopic...

(with previous patch, when the 'subtopic' could not be built, it forced on topicCopy... which "broke" the use case of interest.)

@reidsunderland
Copy link
Member

reidsunderland commented Oct 8, 2024

I posted on Teams, just adding here for proper documentation.

topicCopy works now. But with topicCopy off, the topic that gets posted is v02.post and nothing else. It should be v02.post.data.msc ...

@petersilva
Copy link
Contributor Author

@reidsunderland I looked over the code again... I think it only gets properly set when it is being published, so the log of the component itself would show what you described... is there a way of looking at the next component in the chain ... a subscriber to what was posted? I have the feeling it is published with a good topic, but it will show that truncated one within the component.

@petersilva
Copy link
Contributor Author

The topic is only properly set on publish... based on the relPath.

@reidsunderland
Copy link
Member

I rebased this on the latest dev branch and will finally test it now

@reidsunderland
Copy link
Member

Good news: topicCopy True works correctly, bad news: topicCopy False doesn't work :(

With topicCopy false:

  • the logs say the topic being posted is just v02.post
  • It should match the relPath, e.g. v02.post.data.rwin.observation.atmospheric for relPath data/rwin/observation/atmospheric...
  • I also double checked that this is not just a log issue, the broker says the routing key the messages were published with is v02.post.

With topicCopy true:

  • it works
  • the logs show that the topic is the same as we received from the source, e.g. v02.post.dms-dev.nws.observation.atmospheric, for relPath data/nws/observation/atmospheric/...
  • I confirmed that the broker also gets the correct routing key

I can try to fix it when I have some free time.

@petersilva
Copy link
Contributor Author

This is endlessly confusing, but I think what you are describing is the intended, correct behaviour.
The issue here is that the topic given is invalid, so when topicCopy is off, if the inbound topic is not valid, do not set it. you get v02.post because that comes from the post_topicPrefix.

If the inbound topic were correct, then it would be fine. The DMS input topic is not correct, so there is no correct behaviour possible when topicCopy off I could be wrong... but that´s my guess.

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.

What is correct "topic" processing in sr3?
2 participants