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

sender options inflight test fails if None #1330

Merged
merged 3 commits into from
Dec 6, 2024
Merged

Conversation

petersilva
Copy link
Contributor

fix #1327

just check if option.inflight is not None... if it is None, we don't need to rename after transfer... so it is moot.

Copy link

github-actions bot commented Dec 5, 2024

Test Results

238 tests   235 ✅  1m 33s ⏱️
  1 suites    1 💤
  1 files      2 ❌

For more details on these failures, see this check.

Results for commit d2f2d9a.

♻️ This comment has been updated with latest results.

@petersilva petersilva changed the title options inflight test fails if None sender options inflight test fails if None Dec 5, 2024
@petersilva petersilva marked this pull request as ready for review December 5, 2024 23:08
@petersilva
Copy link
Contributor Author

There is this weird breakage of the flakey on the deve branch for the past week or so...
It is still broken here... thought I had fixed it, but no.

@petersilva petersilva merged commit da9dd37 into development Dec 6, 2024
34 of 60 checks passed
@petersilva petersilva deleted the issue1327 branch December 13, 2024 22:07
@reidsunderland
Copy link
Member

reidsunderland commented Jan 9, 2025

I was taking a close look at this because I was going to use inflight None in a config that I wanted to deploy with 3.00.56 and @andreleblanc11 pointed me to this issue when he reviewed my change.

I noticed a potential problem with the fix.

#=================================
# if umask, check that the protocol supports it ...
#=================================
inflight = options.inflight
if not hasattr(self.proto[self.scheme],
'umask') and options.inflight == 'umask':
logger.warning("%s, umask not supported" % self.scheme)
inflight = None
#=================================
# if renaming used, check that the protocol supports it ...
#=================================
if not hasattr(self.proto[self.scheme], 'rename') and options.inflight:
logger.warning("%s, rename not supported" % self.scheme)
inflight = None

Assuming there is a protocol that does support umask and does not support renaming, the new code will set inflight = None, which would cause inflight umask in the config to be ignored.

But I think FTP is the only protocol that supports umask, and it does support rename as well, so it shouldn't really matter in practice.

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

Successfully merging this pull request may close these issues.

inflight None in sender causes messages to get retried
3 participants