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

sr3 sender fails to post message when post_exchangeSplit is in use and the incoming message does not have a checksum #1322

Closed
reidsunderland opened this issue Dec 4, 2024 · 5 comments · Fixed by #1329
Assignees
Labels
Blocker bad enough that no new release should happen bug Something isn't working Priority 3 - Important worrisome impact... should study carefully regression Broke something that was working before. v3only Only affects v3 branches.

Comments

@reidsunderland
Copy link
Member

Traceback (most recent call last):
-  File "/usr/lib/python3/dist-packages/sarracenia/flow/__init__.py", line 1253, in post
-    p(self.worklist)
-  File "/usr/lib/python3/dist-packages/sarracenia/flowcb/post/message.py", line 40, in post
-    if all_good and hasattr(self.poster,'putNewMessage') and self.poster.putNewMessage(m):
-  File "/usr/lib/python3/dist-packages/sarracenia/moth/amqp.py", line 665, in putNewMessage
-    idx = sum( bytearray(body['identity']['value'],
-KeyError: 'identity'

if ( 'exchangeSplit' in self.o) and self.o['exchangeSplit'] > 1:
# FIXME: assert ( len(self.o['exchange']) == self.o['post_exchangeSplit'] )
# if that isn't true... then there is something wrong... should we check ?
if 'exchangeSplitOverride' in message:
idx = int(message['exchangeSplitOverride'])%len(self.o['exchange'])
else:
idx = sum( bytearray(body['identity']['value'],
'ascii')) % len(self.o['exchange'])

In a sender, we should probably ensure that all messages have an identity. If we receive one that doesn't have identity, we should calculate it.

@reidsunderland reidsunderland added bug Something isn't working Priority 3 - Important worrisome impact... should study carefully Blocker bad enough that no new release should happen labels Dec 4, 2024
@reidsunderland
Copy link
Member Author

I wrote a plugin that seems to fix/workaround the problem:

#!/usr/bin/python3
"""
If identity is missing from the message, calculate and add it.
"""

from sarracenia.flowcb import FlowCB
import sarracenia
import datetime, logging
import os

logger = logging.getLogger(__name__)

class Fix_missing_identity(FlowCB):

    def __init__(self, options) :
        super().__init__(options,logger)

    def after_accept(self, worklist):
        for msg in worklist.incoming:
            if 'identity' not in msg:
                local_path = 'unknown'
                ident = {'method': 'cod', 'value': self.o.identity_method}

                try:
                    local_path = self.o.baseDir + '/' + msg['relPath']
                    local_identity = sarracenia.identity.Identity.factory(self.o.identity_method)
                    local_identity.update_file(local_path)
                    ident = {'method': self.o.identity_method, 'value': local_identity.value}
                except Exception as e:

                    logger.warning(f"Could not calculate identity {e}, setting message identity to {ident}")

                logger.debug(f"set identity to {ident}")

                msg['identity'] = ident

@petersilva
Copy link
Contributor

uh... that was one of the major changes last year... identity became optional. We should fix the code to allow identity to be missing.

@reidsunderland
Copy link
Member Author

  • In a sender, I think we should calculate the checksum if it's missing. We have the file, so we should try to post a message with as much information as possible. @petersilva said it would be best to have an option to disable this for performance reasons
  • We should be able to use post_exchangeSplit without identity in a message. If identity is missing, we should have an alternative (use the filename, relPath, something else?)

@petersilva
Copy link
Contributor

relevant background:

@petersilva petersilva added the regression Broke something that was working before. label Dec 4, 2024
@petersilva petersilva added the v3only Only affects v3 branches. label Dec 5, 2024
@petersilva petersilva self-assigned this Dec 5, 2024
@petersilva
Copy link
Contributor

I will work on this... need to change how post_exchangeSplit works anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker bad enough that no new release should happen bug Something isn't working Priority 3 - Important worrisome impact... should study carefully regression Broke something that was working before. v3only Only affects v3 branches.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants