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

Use python module pysmbc instead of smbclient #74

Closed
wants to merge 1 commit into from

Conversation

spuiuk
Copy link
Collaborator

@spuiuk spuiuk commented Mar 25, 2024

This change introduces a new class SMBClient which is a wrapper around the python module pysmbc. This simplifies access to samba server as demonstrated in the changes to the consistency test which used to use smbclient to access the server.

Note that this changeset consists of only one commit and the rest of the commits are from #73

depends on #73
depends on samba-in-kubernetes/sit-environment#93

)
retstr = scon.simple_read(test_filename)
scon.unlink(test_filename)
del scon
Copy link
Collaborator

@phlogistonjohn phlogistonjohn Mar 25, 2024

Choose a reason for hiding this comment

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

Using del on objects directly is a bit of a (small) code smell in my book. If the point here is to free up resources (the smbc connection?) then it would be better to make SMBClient (also) be a context manager and write the code like:

with SMBClient(host, share, user, pass) as scon:
    scon.simple_write(abc, xyz)

If I am right about the reason for the del and you want tips on making this a context manager, let me know. otherwise: https://realpython.com/python-with-statement/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes. I put the del in to force the connection to be disconnected after each read or write cycle.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a disconnect() method to the SMBClient class. This eventually does a del on the smb context.

We still cannot test this on the sit-environment because we can't install pysmbc on the client machine. But I am considering using another library which is selfcontained and entirely written in python.

@spuiuk
Copy link
Collaborator Author

spuiuk commented Mar 28, 2024

PR #76 opened which uses a different python smb client module which should be easier to install and use on our test systems. Leaving both open as we decide which module to go ahead with.

We use the python module pysmbc to access the SMB server.
The helper class hides the complexities and helps us bypass the
smbclient binary to connect to the remote samba server giving us more
flexibility in the python tests.

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

dpulls bot commented Mar 31, 2024

🎉 All dependencies have been resolved !

@spuiuk
Copy link
Collaborator Author

spuiuk commented Mar 31, 2024

Closing this PR in favour of PR #76

The pysmbc module is the python bindings for the libsmb client library. This means that when installing using pip, we need to recompile the module. This requires additional development packages installed on the test systems. Instead we use the pysmb module which is a pure python module which is easier to use with the test system.

@spuiuk spuiuk closed this Mar 31, 2024
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.

2 participants