Actually, (without trying it) I suppose #1444 has already been solved some time ago (or this change does not solve it) because it looks like we always pass a session to Downloader so this might be a change only affecting tests. I wonder if we should just disallow to pass no session because we don't want to use it in practice or if we want to keep it just to simplify tests?

Argh, I missed one dynamic usage that does not pass a session:

https://github.com/python-poetry/poetry/blob/main/src/poetry/packages/direct_origin.py#L80

However, we could think about changing that.

We do not always pass in a session.

This is what caused #1444 afaict.

I don't think changing it is trivial. And we want to keep allowing the utility function to optionally pass in a session.

I don't think changing it is trivial.

What about creating an Authenticator in DirectOrigin.__init__ and then

        download_func = functools.partial(download_file, session=self._authenticator)
        artifact = self._artifact_cache.get_cached_archive_for_link(
            link, strict=True, download_func=download_func
        )

And we want to keep allowing the utility function to optionally pass in a session.

We probably should avoid using it without passing a session for performance reasons but if you think it makes sense to keep it for convenience, I'm ok with it.

What about creating an Authenticator in DirectOrigin.init

Not opposed to that. But, I do wonder if that actually gains us much since the cache already will be set after the first download and the url never changes.

Reckon we should anyway?

We probably should avoid using it without passing a session for performance reasons but if you think it makes sense to keep it for convenience, I'm ok with it.

I am not so sure there is a discernable performance hit for one-shot downloads. But yes, I do think keeping it for convenience is worthwhile.

But, I do wonder if that actually gains us much since the cache already will be set after the first download and the url never changes.

One DirectOrigin instance is used for all URL downloads during resolving so it's not just one URL. It's probably not that much of a gain but it shouldn't hurt since it will be similar to what Executor is doing.

@radoering made trhe changes, used get_default_authenticator too.

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.