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

why winnows for DMS are different ... v2 vs. sr3 #1017

Open
petersilva opened this issue Apr 16, 2024 · 10 comments
Open

why winnows for DMS are different ... v2 vs. sr3 #1017

petersilva opened this issue Apr 16, 2024 · 10 comments

Comments

@petersilva
Copy link
Contributor

petersilva commented Apr 16, 2024

Summary

if you run sr3 convert winnow/xx on a v2 winnow (or shovel) and then try to run it, it will publish very few, close to nil, messages. There is some additional adjustment needed to get things working properly. Messages from DMS are quite application specific, and v2 has some quirks that make it "interesting"

  • in sr3 winnow configurations, remove nodupe_basis name ... the default algorithm in sr3 works perfectly, and the name one does not.
  • some downstream consumers of sr3 winnow must remove (or not have) subtopic <source>.# ( eg. msc-dms-dev.# )

Symptoms and Troubleshooting

winnow message inbound from DMS looks like this:

2024-04-16 18:09:57,685 [INFO] 48961 sarracenia.moth.amqp _msgRawToDict body: type: <class 'bytes'> (182 bytes) b'20240416180657.536 http://dms-dev2.cmc.ec.gc.ca:8180 /data/msc/observation/atmospheric/surface_weather/ca-1.1-ascii/product_generic_swob-xml-2.0/202404161800/1032731/web/orig/data_60'
2024-04-16 18:09:57,685 [INFO] 48961 sarracenia.moth.amqp _msgRawToDict headers: type: <class 'dict'> (7 elements) {'rename': 'msc.observation.atmospheric.surface_weather.ca-1.1-ascii.product_generic_swob-xml-2.0.202404161800.1032731.web.orig.data_60', 'sundew_extension': 'DMS:CMC:SWOB_CA:XML:', 'parts': '1,10737418240,1,0,0', 'to_clusters': 'DDI,DDSR', 'x-delay': 180000, 'sum': 'a,59f7eccf39937f9cf0b0f61887ef304e', 'source': 'MSC-DMS-DEV'}

so the relPath is on the first line and it ends with the "filename" == "data_60" ... we are using the filename as the key in the duplicate suppression, because ... I don't actually know why... I would think we could use the checksums provided by DMS, but that's a question... anyways... the winnows are currently configured to use the filename.

in v2. the filename value is derive from the value of the rename field: 'msc.observation.atmospheric.surface_weather.ca-1.1-ascii.product_generic_swob-xml-2.0.202404161800.1032731.web.orig.data_60' so THAT does provide uniqueishness. the message is passed without transformation (so that it still works when handed downstream.)

in sr3, this substitution doesn't happen because we are deriving the key from the functional relPath, rather than a filename extracted from it. The relPath can't be modified by a shovel or winnow because the downstream consumer will get the wrong value.

so... what to do...

@petersilva
Copy link
Contributor Author

If I go into nodupe, and explicitly check for the rename field and use it if present...
the DMS use case works. that's one "solution" not sure if the air-quotes are warranted or not... have to examine some other options.

@petersilva
Copy link
Contributor Author

another option is to update things in flow/__init__.py updateFieldsAccepted()

  • set m['retrievePath'] = m['relPath']
  • set m['relPath'] = m['rename']
  • del m['rename']

is that good? It should download to the same place, but if someone is doing accepts,
they don't have the path... rename is just flat... and that is how rename is currently processed... as a flat name replacing the entire relPath.

we could keep the rest of relPath and just replace the name... with mirror off it has the same effect as today. This would allow the same accept/reject clauses to be used by downstream consumers as originally done in v2.

Doing either of those things fixes the nodupe check, but the publish is oddly unaffected. strange... thinking...

@petersilva
Copy link
Contributor Author

another option is... looking at the value of the rename field in DMS messages... it's the the directories in the relpath with / replace by . could just set the nodupe_basis to "path" and it should work fine.... it does. hmm... Are all DMS products built this way?

@petersilva
Copy link
Contributor Author

got confirmation that all DMS messages are built this way, so using path should be fine.
changed the config to reflect that, more testing tommorrow.

@petersilva petersilva changed the title why winnows for DMS don't currently work. why winnows for DMS are different ... v2 vs. sr3 Apr 17, 2024
@petersilva
Copy link
Contributor Author

petersilva commented Apr 17, 2024

another effect, discovered by @andreleblanc11 is that the topic from DMS is not what Sarracenia expects.

example inbound message from DMS:

2024-04-18 00:00:39,014 [INFO] sarracenia.moth.amqp _msgRawToDict body: type: <class 'bytes'> (181 bytes) b'20240417235738.923 http://dms-dev2.cmc.ec.gc.ca:8180 /data/msc/observation/atmospheric/surface_weather/ca-1.2-ascii/product_generic_swob-xml-2.0/202404172356/8205092/aaw/orig/data_1'
2024-04-18 00:00:39,014 [INFO] sarracenia.moth.amqp _msgRawToDict headers: type: <class 'dict'> (7 elements) {'rename': 'msc.observation.atmospheric.surface_weather.ca-1.2-ascii.product_generic_swob-xml-2.0.202404172356.8205092.aaw.orig.data_1', 'sundew_extension': 'DMS:CMC:SWOB_CA:XML:', 'parts': '1,10737418240,1,0,0', 'to_clusters': 'DDI,DDSR', 'x-delay': 180000, 'sum': 'a,902cae93c689473086a2cc61b12f28cc', 'source': 'MSC-DMS-DEV'}

The topic is supposed to be a combination of "v02.post.&ltthe directory tree&gt"
but the DMS ones have a prefix of "msc-dms-dev" ...

v2 topic out-bound: v02.post.msc-dms-dev... unchanged from what is received from DMS
the

the sr3 one makes it conform to sarracenia expects, matching the path, so it removes the msc-dms-dev.

2024-04-17 21:19:39,349 [INFO] sarracenia.flowcb.log after_post posted to exchange: xpublic topic: v02.post.20240417.MSC-DMS-DEV.NSWOB.wmo.fm13.21 a file with baseUrl: http://ddsr-cmc-dev02.cmc.ec.gc.ca/ relPath: 20240417/MSC-DMS-DEV/NSWOB/wmo/fm13/21/NSWOB_wmo.observation.atmospheric.surface_weather.fm13_buoy-2.0-ascii.product-ninjo_nswob_xml-1.0.20240417210000000.42013.rrm_DMS_20240417210000000.xml:DMS:CMC:NSWOB:XML:20240417211938

To get the same as v2 we could add post_topicPrefix v02.post.msc-dms-dev to the winnow config.

@petersilva
Copy link
Contributor Author

Just changed the summary at the top:

  • nodupe_basis path ... is actually the default, so DMS works perfectly with the default duplicate suppression... no settings needed.
  • to clarify the sr3 change in the topic (see previous comment) is an example of Postel's Maxim: "be liberal in what you accept, conservative in what you send"... the topic tree received does not match Sarracenia expectations, so it is altered on send to match what Sarracenia expects. I'm puzzling over whether that is a good thing or not, but I am tending toward yeah, it's a good thing the way it is.

@petersilva
Copy link
Contributor Author

petersilva commented Apr 22, 2024

Summarizing discussion:

  • people are surprised that shovel & winnow modify the messages they filter. People expect the messages to pass through un-changed (so a violation of POLA - Principle of Least Astonishment)
  • for plugins to work the same in shovel, winnow and components that do download, we need for fields like "new_file" and "new_dir" to be honoured when they are used in either components.
  • so if we pass the messages unfiltered, we have plugins that work different ways depending on what component they are in... in download=off case, they modify the original message, in download=on case, they modify the download fields.
  • use of either method in the wrong case will result in the plugin's work being ignored (using a shovel/winnow plugin on a subscribe/sarra ... the name change will be ignored. and in the reverse case, the fields would not be there, so it would likely crash or do nothing.) (This is how v2 worked and was considered BAD/confusing )

There are also settings like post_baseURL that need to be respected, and those involve modifying the message in flight, and would again require two different cases, changing different fields depending on self.o.download.

So the real ask is to pass the message through, and re-build it just as it was... It isn't really to pass it through un-modified.

We talked about using a more elaborate topicPrefix, but the objection was that if the topic isn't fixed ( v02.post.msc-dms-dev, v02.post.msc-dms-stage ) ... how to allow for that...

Another option would be to validate the topic on receipt. If it matches the sarracenia norms, then do nothing, normal processing should be correct. If it does not, then build a topic header as an over-ride. ... yeah that might work... thinking...

@petersilva
Copy link
Contributor Author

OK, I coded it up so:

  • if the topic in the incoming message doesn't match what sarracenia expects, then rather the de-composing it in a standard way for it to be re-constructed on publish, create and "topic" field which acts as an override.

the problem with this is that... when download=True, the topic still won't be overwritten. So I guess I need to delete the topic override when download=True to allow construction of correct topics for publish?

Can still override the topic with after_work(), or post() ... hmm... does that make sense? Maybe post_topic construction is more complicated?

@petersilva
Copy link
Contributor Author

So... now I'm thinking topicPreserve as an option, set to True by default for shovels and winnows, and False for things that download....

@petersilva
Copy link
Contributor Author

Now I'm having philosophical second thoughts: maybe the whole way "topic" is handled is just weird. we currently parse the inbound topic into a "subtopic" and keep the "topicPrefix" separately. if the topic provided doesn't match the topicPrefix (not sure how that would happen.) then the chop of subtopic is wrong.

The subtopic is a list because the topic separate varies by protocol... e.g. if reading from amqp and publishing to mqtt the separator is . on input and / on output. so turning it into a list makes some sense... This is turning into it's own issue.

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

No branches or pull requests

1 participant