Sensor not detecting new items in Inbox about stackstorm-msexchange HOT 24 CLOSED

So what's probably needed is to update these pieces:

self._set_last_date(payload['datetime_received'])
...
 def _set_last_date(self, last_date):
        self._last_date = time.strftime('%Y-%m-%dT%H:%M:%S', last_date)
        self._sensor_service.set_value(name=self._store_key,
                                       value=self._last_date)

And make sure that this bit time.strftime('%Y-%m-%dT%H:%M:%S', last_date) is getting last_date in the format it expects.

from stackstorm-msexchange.

We use this pack internally for a number of different things (we currently have an open PR on this pack with some additional enhancement added to the Sensor to hopefully can be merged at some point), but I would have to look at the code again to see if there would be any impact to removing the datetime check. My guess is there will not be any issue (aside from the possibility of duplicate triggers if emails are reset manually to unread) with that but I will need to confirm.

from stackstorm-msexchange.

msexchange.search_items only filters by subject if one is given. It does not filter by timestamp or read/unread status.

The sensor code does do filtering by timestamp and 'read' status.

What happens if you do an interactive Python session, use msexchangelib to connect to that account, and search for items, with and without timestamp & read/unread filtering?

from stackstorm-msexchange.

I wonder if the time stamp code is wrong. Should be easy to test out.

from stackstorm-msexchange.

@ngesnetworks is your system time zone the same as the time zone in your /opt/stackstorm/configs/msexchange.yaml file?

from stackstorm-msexchange.

@LindsayHill For the msexchange.yaml file I originally had American/Chicago, but I just changed it to UTC and registered the config. After doing so I ran sudo /opt/stackstorm/st2/bin/st2sensorcontainer --config-file=/etc/st2/st2.conf --sensor-ref=msexchange.ItemSensor and the sensor worked. But I am now seeing the following error message.

Traceback (most recent call last):
  File "/opt/stackstorm/st2/local/lib/python2.7/site-packages/st2reactor/container/sensor_wrapper.py", line 250, in run
    self._sensor_instance.run()
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/st2reactor/sensor/base.py", line 105, in run
    self.poll()
  File "/opt/stackstorm/packs/msexchange/sensors/item_sensor.py", line 55, in poll
    self._set_last_date(payload['datetime_received'])
  File "/opt/stackstorm/packs/msexchange/sensors/item_sensor.py", line 81, in _set_last_date
    self._last_date = time.strftime('%Y-%m-%dT%H:%M:%S', last_date)
TypeError: argument must be 9-item sequence, not EWSDateTime

I also tested out just simply removing the Timezone from the yaml file and it showed the same issue with the EWSDateTime.

2018-08-01 09:12:15,172 ERROR [-]
2018-08-01 09:12:15,172 ERROR [-] obj.run()
2018-08-01 09:12:15,173 ERROR [-]   File "/opt/stackstorm/st2/local/lib/python2.7/site-packages/st2reactor/container/sensor_wrapper.py", line 256, in run
2018-08-01 09:12:15,173 ERROR [-]
2018-08-01 09:12:15,174 ERROR [-] raise Exception(msg)
2018-08-01 09:12:15,174 ERROR [-] Exception
2018-08-01 09:12:15,174 ERROR [-] :
2018-08-01 09:12:15,175 ERROR [-] Sensor "ItemSensor" run method raised an exception: argument must be 9-item sequence, not EWSDateTime.
2018-08-01 09:12:15,175 ERROR [-]
2018-08-01 09:12:15,176 INFO [-] Stopping trigger watcher

from stackstorm-msexchange.

What happens if you set your time zone to be the same as your server time zone?

from stackstorm-msexchange.

My timezone and the server timezone are the same.

from stackstorm-msexchange.

What happens if you do an interactive Python session, use msexchangelib to connect to that account, and search for items, with and without timestamp & read/unread filtering?

from stackstorm-msexchange.

I used the Teaser from the library to get connected up. https://github.com/ecederstrand/exchangelib#teaser

credentials = Credentials('[email protected]', 'topsecret')
account = Account('[email protected]', credentials=credentials, autodiscover=True)

for item in account.inbox.all().order_by('-datetime_received')[:100]:
    print(item.subject, item.sender, item.datetime_received)

I was able to pull in the following.

('testing', Mailbox(name='****************', email_address='*************', routing_type='SMTP', mailbox_type='Mailbox', item_id=None), EWSDateTime(2018, 8, 1, 18, 13, 10, tzinfo=<UTC>))

I used a simple print(account.inbox.unread_count) and was able to get the correct read/unread count.
Then I used for item in account.inbox.all(): print(item.datetime_received) and was able to get the times associated with it. The times that are pulled through are UTC (since I'm using datetime) I see 18:37:16+00:00, which is as expected. When I use it with the original teaser code I see EWSDateTime(2018, 8, 1, 18, 13, 10, tzinfo=<UTC>).

from stackstorm-msexchange.

This is still an issue for first time sensor start up.

As the sensor comes online the stored date is None which then defaults to datetime.now(). This never gets stored so the sensor is never able to pick anything up.

Stored date is None -> start_date is set to datetime.now() - any email previous to now() is never detected.

Probably need to add set last date as well so that we can get the triggers moving forward.

from stackstorm-msexchange.

This is still an issue for first time sensor start up.

As the sensor comes online the stored date is None which then defaults to datetime.now(). This never gets stored so the sensor is never able to pick anything up.

Stored date is None -> start_date is set to datetime.now() - any email previous to now() is never detected.

Probably need to add set last date as well so that we can get the triggers moving forward.

stackstorm-msexchange/sensors/item_sensor.py

Lines 47 to 49 in 577ce37

	if not stored_date:
	stored_date = datetime.now()
	start_date = self._timezone.localize(EWSDateTime.from_datetime(stored_date))

Maybe the bigger question here is do we actually need to use a stored datetime? Since the sensor filters out all non-read items and then marks any new items as read it wont see those items again on the next execution.

from stackstorm-msexchange.

I'm happy to submit a PR on this if we can have a discussion on the direction to go.

from stackstorm-msexchange.

Maybe the bigger question here is do we actually need to use a stored datetime? Since the sensor filters out all non-read items and then marks any new items as read it wont see those items again on the next execution.

I don't use this pack, but looking at the logic I would agree for the most part the date check could be removed, and it would resolve this issue and simplify logic.

There is one use case that I think would differ:

1. We get in an email
2. Sensor picks it up (and currently we note the time)
3. If we at a later point mark that email as unread....
   Then currently that email wouldn't get picked up again, but if we change the logic to remove the date checking then the email would get sent in a second time.

The imap filter in the email pack, however just uses your simpler logic which is to just return the unread rather than keep a tab on the last updated.

Would be interesting to hear back from anyone actively using this pack, to see if this change in logic would have any detrimental affect on them.

As the alternative would be that if its not set, then don't filter by date after done the unread, so only do that if we have a stored_date set.

from stackstorm-msexchange.

Currently the sensor will only pick up unread emails after x datetime. So the only way the same email might be picked up is if the sensor executes again before the previous one finishes and I'm not even sure if that's possible with st2?

This pack also does not do any sorting, and I'm not sure what the default sorting method is. So really the last stored datetime value is going to be the last item in the list depending on how its sorted it could be asc or desc and we'd end up storing the wrong datetime anyways.

Personally as of today I've cloned this repo and removed the datetime store all together and it works fine. It checks the folder for any unread emails and then marks them read. Just in my experience with other commercial software the way I've modified this pack without the datetime store is how they also work - checking for unread emails, not after a certain time period.

Here you can see that one check found one item and the next found zero. The email that it marked unread is still in that same mailbox.

2021-11-02 14:45:24,377 140248160129096 INFO item_sensor [-] Found 1 items
2021-11-02 14:46:27,145 140248160129096 INFO item_sensor [-] Found 0 items

IMHO the way I see it we have two options:

1. Remove datetime all together (which is what I'm in favor of), or
2. Add self._set_last_date(stored_date) after line 48 in order to store an initial value on first time use. This way at the very least there will be a 1m window that emails can be picked up by the sensor.

In the end I think there's just too many issues to hinge it on the datetime. In the grand scheme of unreliable email a 1m window isn't really enough. Email delays, emails that are scheduled can actually have a datetime stamp of the template and not the actual email sent, etc.

from stackstorm-msexchange.

What I meant was if we remove the date time check, then in the scenario mentioned - this would cause us to get the message a 2nd time if the user marks it as unread.

however I would have expected in most scenarios we would be monitoring accounts where someone wasn’t going to do that.

And as you said delays on timing would mess it up - and I would feel they would be more likely to happen then read emails on an account being monitored being marked as unread.

I am also in favour of removing the date time check.

from stackstorm-msexchange.

@amanda11 So looking at #17 a lot of the datetime stuff is fixed there and made optional. However, it removes the is_read flag which I think shouldn't be the case or at the very least made optional. What's nice about it is it allows you to do relative datetime searches which makes good sense.

Anyways, this PR is 5 months old and seems like it was abandoned and not merged for some reason. Although the request for a fix wasn't made until beginning of last month. Can we move forward with this PR - I can work on any fixes if they're not meeting requirements.

from stackstorm-msexchange.

@amanda11 So looking at #17 a lot of the datetime stuff is fixed there and made optional. However, it removes the is_read flag which I think shouldn't be the case or at the very least made optional. What's nice about it is it allows you to do relative datetime searches which makes good sense.

Anyways, this PR is 5 months old and seems like it was abandoned and not merged for some reason. Although the request for a fix wasn't made until beginning of last month. Can we move forward with this PR - I can work on any fixes if they're not meeting requirements.

Actually I'm mistaken it doesn't change the sensor, just the actions when searching for items. Still, the PR is a pretty good update to this package. With the dateutil included with that PR making similar changes to the sensor would be trivial.

from stackstorm-msexchange.

@jamesdreid Did you get chance to see if removing the dateutil logic from the sensor would cause any problems?
@minsis Are you thinking now of making the sensor so that the check on pulling only from last date optional? Or still going for the remove the date logic?
I'm not sure this fix is reliant on #17 getting merged?

Thanks for the review comments on #17, once those are addressed I think we can move forward on that PR getting merged.

from stackstorm-msexchange.

Sorry I got confused; I was thinking dateutil was dateparser library. So I guess my plan is back to removing it which doesn't rely on #17. But, #17 does make changes to the item_sensory.py so there would probably be merge conflicts if I make changes now.

from stackstorm-msexchange.

After discussing internally, the general consensus was there would be no impact. We should be fine to go ahead with the dateutil logic from the sensor, but I would prefer it was done AFTER #17 was merged (or fully rejected with reason) as it was submitted by an associate of mine and is used internally for extracting attachments from emails processed by the sensor and is an integral component to some of our automation workflows. If #17 is not merged, we will just continue to use our forked version and the dateutil logic will make no difference.

from stackstorm-msexchange.

So what are next steps to getting #17 merged?

from stackstorm-msexchange.

@minsis #17 merged now.

from stackstorm-msexchange.

Hi @amanda11,

Sorry for the gross delay here, been trying to keep my head above water! Anyways, PR #21 fixes the issue of datetime check for the item sensor.

from stackstorm-msexchange.