Skip to content

Conversation

@spuiuk
Copy link
Collaborator

@spuiuk spuiuk commented Oct 28, 2025

net conf import is unable to use the STDIN to import the configuration when the generated smb.conf is large. Use a tempfile instead as an argument to net conf import.

Resolves: rhbz#2400102

@spuiuk spuiuk requested a review from phlogistonjohn October 28, 2025 18:01
@spuiuk
Copy link
Collaborator Author

spuiuk commented Oct 28, 2025

Flake8 failures CI/test (fedora-latest) seem to be unrelated.

flake8: install_package> python -I -m pip install --force-reinstall --no-deps /var/tmp/build/sambacc/.tox/.tmp/package/1/sambacc-0.8.dev6+gfd0a3b236.tar.gz
flake8: commands[0]> flake8 --exclude sambacc/grpc/generated sambacc tests
sambacc/commands/cli.py:28:1: F401 'sambacc.opener' imported but unused
sambacc/commands/cli.py:227:5: F811 redefinition of unused 'opener' from line 28
flake8: exit 1 (0.80 seconds) /var/tmp/build/sambacc> flake8 --exclude sambacc/grpc/generated sambacc tests pid=94
flake8: FAIL ✖ in 8.24 seconds

Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks.

@phlogistonjohn
Copy link
Collaborator

Oh, it's fedora 43. It just got released.

@phlogistonjohn
Copy link
Collaborator

#179 for a fix for the ci failure

@anoopcs9
Copy link
Collaborator

anoopcs9 commented Oct 29, 2025

@Mergifyio rebase

net conf import is unable to use the STDIN to import the configuration
when the generated smb.conf is large. Use a tempfile instead as an
argument to net conf import.

Resolves: rhbz#2400102

Signed-off-by: Sachin Prabhu <[email protected]>
@mergify
Copy link

mergify bot commented Oct 29, 2025

rebase

✅ Branch has been successfully rebased

Copy link
Collaborator

@anoopcs9 anoopcs9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with the changes although we are yet to root cause the real problem with net conf import via STDIN. Should we investigate after filing a bz?

@xhernandez
Copy link
Collaborator

I'm good with the changes although we are yet to root cause the real problem with net conf import via STDIN. Should we investigate after filing a bz?

The change looks good, but I think we should know why it fails. Specially because it's a silent failure, which is not good.

@spuiuk
Copy link
Collaborator Author

spuiuk commented Oct 29, 2025

Yes. I think this should be reported in a bz in samba upstream since that is where we have a bug. This problem could affect other users as well. For Ceph-SMB, I would say this is fixed and move on.

@mergify mergify bot added the queued label Oct 31, 2025
@mergify mergify bot merged commit 83c606c into samba-in-kubernetes:master Oct 31, 2025
11 checks passed
@mergify mergify bot removed the queued label Oct 31, 2025
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.

4 participants