O365 / python-o365

A simple python library to interact with Microsoft Graph and Office 365 API
Apache License 2.0
1.65k stars 419 forks source link

2.0.33 : 'MSGraphProtocol' object has no attribute 'keyword_data_store' #1053

Closed stezz closed 7 months ago

stezz commented 7 months ago

Testing the new release and I am getting this error:

    self.protocol = MSGraphProtocol()
                    ^^^^^^^^^^^^^^^^^^^
  File "development/python-o365/O365/connection.py", line 242, in __init__
    super().__init__(
  File "development/python-o365/O365/connection.py", line 114, in __init__
    self.timezone = timezone or get_localzone()
    ^^^^^^^^^^^^^
  File "development/python-o365/O365/connection.py", line 261, in timezone
    self.keyword_data_store['prefer_timezone_header'] = f'outlook.timezone="{get_windows_tz(self._timezone)}"'
    ^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'MSGraphProtocol' object has no attribute 'keyword_data_store'

the problem seems to be here:

class MSGraphProtocol(Protocol):
....
        self.keyword_data_store['prefer_timezone_header'] = f'outlook.timezone="{get_windows_tz(self._timezone)}"'
        self.max_top_value = 999  # Max $top parameter value

    @Protocol.timezone.setter
    def timezone(self, timezone: Union[str, ZoneInfo]):
        super()._update_timezone(timezone)
        >>> self.keyword_data_store['prefer_timezone_header'] = f'outlook.timezone="{get_windows_tz(self._timezone)}"' <<<
stezz commented 7 months ago

Maybe @Invincibear might have some insight into this?

Invincibear commented 7 months ago

I haven't touched that part of the codebase, but sounds like https://github.com/O365/python-o365/issues/1051 would fix it, eventually. I would look at the Protocol class to see if it defines keyword_data_store, it looks like the code is assigning that dict a value but was the dict ever created before adding an item to it?

alejcas commented 7 months ago

That is probably my fault… I have to look into it a bit. I’ve run some test and all worked, let me see. I’ll report back

stezz commented 7 months ago

Thanks @alejcas. No worries and no rush, I’ll hold back the update in the meantime ;)

stezz commented 7 months ago

Ps. Probably worth to mention that I’m not using the “user” account but the auth_flow_type=‘credentials’

alejcas commented 7 months ago

Fixed!

Can you try again? I'll release a new version if all works

stezz commented 7 months ago

Almost there:

In [1]: import os
   ...: from O365 import Account, MSGraphProtocol
   ...: protocol = MSGraphProtocol(api_version="beta") 
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[1], line 5
      3 from urllib import parse
      4 import csv
----> 5 protocol = MSGraphProtocol(api_version="beta")  # MSGraphProtocol defaults to v1.0 api version

File ~/development/python-o365/O365/connection.py:245, in MSGraphProtocol.__init__(self, api_version, default_resource, **kwargs)
    243 self.keyword_data_store['file_attachment_type'] = '#microsoft.graph.fileAttachment'
    244 self.keyword_data_store['item_attachment_type'] = '#microsoft.graph.itemAttachment'
--> 245 self.keyword_data_store['prefer_timezone_header'] = f'outlook.timezone="{get_windows_tz(self._timezone)}"'
    246 self.max_top_value = 999

File ~/development/python-o365/O365/utils/windows_tz.py:635, in get_windows_tz(iana_tz)
    628 def get_windows_tz(iana_tz: ZoneInfo) -> str:
    629     """ Returns a valid windows TimeZone from a given pytz TimeZone
    630     (Iana/Olson Timezones)
    631     Note: Windows Timezones are SHIT!... no ... really THEY ARE
    632     HOLY FUCKING SHIT!.
    633     """
    634     timezone = IANA_TO_WIN.get(
--> 635         iana_tz.key if isinstance(iana_tz, tzinfo) else iana_tz)
    636     if timezone is None:
    637         raise ZoneInfoNotFoundError(f"Can't find Iana timezone: {iana_tz.key}")

AttributeError: '_PytzShimTimezone' object has no attribute 'key'

python-o365 is using tzlocal methods, which is using the (now deprecated) pytz which returns a <class 'pytz_deprecation_shim._impl._PytzShimTimezone'> that doesn't get handled well by the code.

You might want to stop using pytz there and do something like:

import time
[...]
self._timezone: ZoneInfo = time.localtime().tm_zone

as time.localtime().tm_zone returns CET strings that the code should understand:

time.localtime().tm_zone
'EET'
type(time.localtime().tm_zone)
str

And this should work in Windows as well (I hope at least). [I am on a Mac and can't test that]

stezz commented 7 months ago

Ok if I you use time.localtime().tm_zone this is what happens later on:

File ~/development/python-o365/O365/drive.py:1609, in Drive._update_data(self, data)
   1607 local_tz = self.protocol.timezone
   1608 print(local_tz)
-> 1609 self.created = parse(created).astimezone(local_tz) if created else None
   1610 self.modified = parse(modified).astimezone(local_tz) if modified else None

TypeError: tzinfo argument must be None or of a tzinfo subclass, not type 'str'

Any chance you can get away without using this get_windows_tz function if you use that tm_zone on Windows ?

Even if I try to do

import time
import datetime
[...]
self._timezone: ZoneInfo = datetime.tzinfo(time.localtime().tm_zone)

Then later on I get in trouble here:

File ~/development/python-o365/O365/utils/windows_tz.py:635, in get_windows_tz(iana_tz)
    628 def get_windows_tz(iana_tz: ZoneInfo) -> str:
    629     """ Returns a valid windows TimeZone from a given pytz TimeZone
    630     (Iana/Olson Timezones)
    631     Note: Windows Timezones are SHIT!... no ... really THEY ARE
    632     HOLY FUCKING SHIT!.
    633     """
    634     timezone = IANA_TO_WIN.get(
--> 635         iana_tz.key if isinstance(iana_tz, tzinfo) else iana_tz)
    636     if timezone is None:
    637         raise ZoneInfoNotFoundError(f"Can't find Iana timezone: {iana_tz.key}")

AttributeError: 'datetime.tzinfo' object has no attribute 'key'
stezz commented 7 months ago

connection.patch Oh actually this works:

self._timezone: ZoneInfo = ZoneInfo(time.localtime().tm_zone)

And get_windows_tz complies with it:

from O365 import Account, MSGraphProtocol
protocol = MSGraphProtocol(api_version="beta")
from O365.utils import get_windows_tz
get_windows_tz(protocol.timezone)

'GTB Standard Time'

So that is your fix apparently ;)

alejcas commented 7 months ago

You might want to stop using pytz there and do something like:

We already dropped support for pytz. Are you using it?

alejcas commented 7 months ago

So that is your fix apparently ;)

I don't see the fix...

self._timezone: ZoneInfo = ZoneInfo(time.localtime().tm_zone)

Where and why are you using this?

In protocol class we do:

self._timezone: ZoneInfo = get_localzone()

get_localzone() from tzlocal already returns a ZoneInfo instance.

We don't use the time module. Maybe you are referring to your own code.

Guess it's all working as it is.

Thanks

stezz commented 7 months ago

@alejcas apologies for lack of clarity.

We already dropped support for pytz. Are you using it?

tzlocal.get_localzone is still using pytz under the hood:

import pytz_deprecation_shim as pds
...
def get_localzone():
    """Returns the zoneinfo-based tzinfo object that matches the Windows-configured timezone."""

    global _cache_tz
    if _cache_tz is None:
        _cache_tz = pds.timezone(get_localzone_name())

    if not utils._tz_name_from_env():
        # If the timezone does NOT come from a TZ environment variable,
        # verify that it's correct. If it's from the environment,
        # we accept it, this is so you can run tests with different timezones.
        utils.assert_tz_offset(_cache_tz)

    return _cache_tz

So you might have dropped pytz everywhere else in your code, but you are using it through tzlocal.get_localzone

The fix is in the connection.patch patchfile attached to this comment

Here again -> connection.patch

We don't use the time module. Maybe you are referring to your own code.

I know that you are not, I am suggesting you do instead of tzlocal

Guess it's all working as it is.

Unfortunately it is not working as it is.

alejcas commented 7 months ago

tzlocal dropped pytz completely.

I think you are using an old version

We now enforce tzlocal >=5.0

https://github.com/regebro/tzlocal?tab=readme-ov-file#api-change

stezz commented 7 months ago

Ah I see! As I am testing from git, I didn't update the deps.

Let me test again

stezz commented 7 months ago

Awesome!

Tested with

  • Updating tzdata (2023.4 -> 2024.1)
  • Updating tzlocal (5.0 -> 5.2)

And it all works like a charm. Thanks ;)

alejcas commented 7 months ago

ok, closing this and will release next version soon