devopshq / artifactory

dohq-artifactory: a Python client for Artifactory
https://devopshq.github.io/artifactory/
MIT License
273 stars 144 forks source link

Calling `ArtifactoryPath.aql()` does not respect the timeout passed to path #447

Open davebirch opened 5 months ago

davebirch commented 5 months ago

dohq-artifactory version: 0.10.0

For some reason, unlike all of the rest of the HTTP requests to artifactory, the call to ArttifactoryPath.aql() seem to directly call self.session.post(), without passing in any of the usual kwargs set on the ArtifactoyPath (verify, cert and more importantly timeout) or going through any of the accessor methods that other calls do.

I'm currently looking at having to extend the request.Session that we pass to your library in order to handle a case where some AQL queries intermittently hang forever due to this.

Whilst I'm happy to do that as an interim solution, it would be nice if this could be treated like every other request that this library makes and pass the kwargs from ArtifactoryPath through to the session. Is there some design decision why this wasn't the case for AQL, or did it just get missed?

Additionally, there's some complication currently with choosing a reasonable timeout value for other requests, given that some of those requests (metadata queries etc) naturally require a very short timeout, whilst other requests (actually downloading the file) require a much longer timeout that I'm again currently planning to handle in my codebase, but would prefer to just be able to simply set in ArtifactoryPath.

Not sure if I should raise as a separate issue, but would be really nice to have timeout used for the vast majority of quick, responsive queries and a separate download_timeout (or similar) which could be set to a larger value for the other use-case where we're downloding the actual artifact data itself, which might be a large file that we don't expect to complete in the few seconds most of the REST/AQL based queries would.

Happy enough I understand both these issues enough to probably be able to put a PR together for both of these issues if you're happy for that, but thought I'd raise it and see what the feedback is on the ideas first before doing any work.