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

add tests for local and smb flavors of PyFilesystemContentsManager #16

Merged
merged 27 commits into from
Apr 15, 2020

Conversation

telamonian
Copy link
Collaborator

currently just has tests for the local flavor

telamonian and others added 10 commits March 24, 2020 22:01
- acheived by deleting files in a bucket from leaf to roots. Turns out it's ridiculously expensive (at least with s3proxy) to delete a non-empty s3 dir
- by default, netbiosd is always running on osx. This blocks the netbios name port (137/udp) and most of the functionality of the nmbd netbios demon that run on the samba docker. Work around netbiosd by using a different name_port for our samba-related activities
@telamonian telamonian force-pushed the smb_local_ci_tests branch 2 times, most recently from 8b0fddb to 3ca2184 Compare April 1, 2020 11:16
@telamonian
Copy link
Collaborator Author

telamonian commented Apr 1, 2020

I got the samba flavor of PyFilesystemContentsManager working on both OSX and Linux, and I added relevant CI tests.

Presumably it will work on Windows as well. It'll be a bit tricky, though. On OSX/Linux I set up a docker that exposes a samba service during the CI. This isn't really an option on Windows, and probably isn't what we want anyway. Instead, it would make a much better test if we can get PyFilesystemContentsManager working with the builtin Windows file sharing framework.

What makes this difficult is that I don't currently have a Windows dev environment set up, so I can't test anything locally. What I'll have to figure out is how to set up a Windows share folder from scratch using only the command line (ie what's available to me on the standard Azure Windows host).

@timkpaine Any ideas?

@telamonian telamonian force-pushed the smb_local_ci_tests branch 4 times, most recently from eba9a76 to 80c3eaf Compare April 14, 2020 08:15
@telamonian telamonian force-pushed the smb_local_ci_tests branch 2 times, most recently from 0db98eb to 3d01a94 Compare April 14, 2020 17:21
@telamonian
Copy link
Collaborator Author

Whelp, the Azure Windows build is still borked, but there's now a complete working example of jupyter-fs + samba on Windows over at Appveyor:

https://ci.appveyor.com/project/telamonian/jupyter-fs/builds/32177461

- also enabled linting and js tests in appveyor
@telamonian telamonian closed this Apr 14, 2020
@telamonian telamonian reopened this Apr 14, 2020
@telamonian
Copy link
Collaborator Author

@timkpaine I've done all that I planned for this PR and all of the tests are now passing, including the windows ci on appveyor:

https://ci.appveyor.com/project/telamonian/jupyter-fs/builds/32184243

For now, the windows ci on azure has been disabled.

This PR is now ready for review!

Makefile Show resolved Hide resolved
setup.py Show resolved Hide resolved
@timkpaine
Copy link
Collaborator

Merge at will!

@telamonian
Copy link
Collaborator Author

Turns out I can't merge:

Required statuses must pass before merging
All required statuses and check runs on this pull request must run successfully to enable automatic merging. 

FML

The tests are passing, but Github is being janky and the statuses aren't collecting correctly. @timkpaine Do you have enough access to override and merge?

@timkpaine timkpaine closed this Apr 15, 2020
@timkpaine timkpaine reopened this Apr 15, 2020
@telamonian telamonian closed this Apr 15, 2020
@timkpaine timkpaine reopened this Apr 15, 2020
@timkpaine
Copy link
Collaborator

timkpaine commented Apr 15, 2020

@telamonian I didn't merge I just tried to get it to retrigger the checks

@telamonian
Copy link
Collaborator Author

@timkpaine I know. I was trying to see if I left it closed for an hour and then reopened if that would make any difference with the statuses bug. But it probably wasn't going to anyway

@timkpaine
Copy link
Collaborator

@telamonian lol

@timkpaine timkpaine merged commit 1de9579 into jpmorganchase:master Apr 15, 2020
@telamonian telamonian changed the title [WIP] add tests for local and smb flavors of PyFilesystemContentsManager add tests for local and smb flavors of PyFilesystemContentsManager Apr 15, 2020
@timkpaine
Copy link
Collaborator

@telamonian looks like it worked after merge

@telamonian telamonian added this to the 0.0.2 milestone May 6, 2020
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.

None yet

3 participants