Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SMB: errors ignored when connecting #1571

Open
frafra opened this issue Apr 12, 2024 · 6 comments
Open

SMB: errors ignored when connecting #1571

frafra opened this issue Apr 12, 2024 · 6 comments

Comments

@frafra
Copy link

frafra commented Apr 12, 2024

The system tries to connect 5 times in a row, ignoring all errors. Since the for loop has no else branch, the function does not raise any error when all tentative failed.

for _ in range(5):
try:
smbclient.register_session(
self.host,
username=self.username,
password=self.password,
port=self._port,
encrypt=self.encrypt,
connection_timeout=self.timeout,
)
break
except Exception:
time.sleep(0.1)

@martindurant
Copy link
Member

You are quite right! Would you like to implement a fix?

@frafra
Copy link
Author

frafra commented Apr 12, 2024

I would suggest dropping the multiple tentative: if the connection is unstable, it should be up to the user to rely on some automatic retry mechanism such as tenacity or backoff. What is your opinion on that?

@martindurant
Copy link
Member

I don't know what the user's workflow might be or how they would apply those tools, but in our own CI pipelines, SMB is unstable for some reason, and this loop makes tests fail less often.

@frafra
Copy link
Author

frafra commented Apr 12, 2024

Since it is a problem with how the CI is configured, I would suggest applying such workaround to the CI only,

@martindurant
Copy link
Member

Plenty of other implementations have retry loops for connection errors, why do you think it's a problem here?

@frafra
Copy link
Author

frafra commented Apr 15, 2024

Well, I am not familiar with SMB, but generally a failing connection hints at some underlying issues. If SMB is intrinsically unstable, it would be nice to have a reference to in the code (why wait 0.1 seconds? why trying just 5 times?).
Automatic retry can be a feature whenever it is needed, but the developer should be in control of that; otherwise one could end up with a connection which usually takes 5 tentative, and sometimes 6, so it would apparently fail randomly without even having a warning logged by the library.

Is SMB supposed to randomly fail? Is it a documented behavior? Or is it a common misconfiguration or glitch that the developer/user should be made aware of?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants