airbytehq / PyAirbyte

PyAirbyte brings the power of Airbyte to every Python developer.
https://docs.airbyte.com/pyairbyte
Other
231 stars 41 forks source link

rmtree error on Windows #424

Open ZmeiGorynych opened 4 weeks ago

ZmeiGorynych commented 4 weeks ago

When running the below code on Windows, I get the following error:

Original exception was:
Traceback (most recent call last):
  File "C:\Users\Egor\.conda\envs\motleycrew3.11\Lib\shutil.py", line 632, in _rmtree_unsafe
    os.unlink(fullname)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\Egor\\AppData\\Local\\Temp\\tmpuk1rvs42\\channels.sqlite'

Full console output here: slack-connector-console-output.txt

This can be fixed by changing line 120 in entrypoint.py to

with tempfile.TemporaryDirectory(ignore_cleanup_errors=True) as temp_dir:

I ran into the same error with other projects, seems to be an shutil issue on Windows.

Shall I submit a PR for that?

Here's the code to reproduce

import airbyte as ab

source = ab.get_source(
    "source-slack",
    config={
        "api_token": "my_token",
        "start_date": "2024-10-14T00:00:00Z",
        "lookback_window": 1,
        "join_channels": True,  # Will auto-join all channels the bot has access to
        # "channel_filter": ["test-slack-ingestion"],
    },
    install_if_missing=True,
)
source.check()
source.select_streams(
    [
        "channels",
        "channel_members",
        "channel_messages",
        # "threads",
    ]
)

result = source.read()
aaronsteers commented 3 weeks ago

@ZmeiGorynych - Thanks for raising. I've opened a PR here:

Does this look correct, and can you confirm that this fixes the issue on your side?

PyAirbyte itself has run into this issue, and we have Windows tests in place now to confirm it doesn't reapear. However, the connectors and the CDK are only tested on Linux. Windows compat is admittedly a lower priority on the connector and CDK-side, but this one seems like an easy enough fix that I'd like to see it through.

As a workaround, if you have a docker runtime locally, you could use docker_image=True in get_source() as a workaround, but either way, I do think this is a worthwhile fix to implement.

ZmeiGorynych commented 3 weeks ago

Yes, that's perfect, thanks!

aaronsteers commented 3 weeks ago

I'll reopen so we can keep the conversation/tracking going if needed.

(This issue should still auto-close when that merges.)

EgorKraevTransferwise commented 3 weeks ago

By the way, docker_image=True breaks on Windows as well :)