Skip to content
This repository was archived by the owner on Oct 3, 2020. It is now read-only.

Fix Windows paths by using posixpath for URL building #58

Closed
wants to merge 3 commits into from

Conversation

jbw
Copy link

@jbw jbw commented Mar 26, 2020

This PR fixes a bug in Windows where api_kwargs builds an invalid URL.

#57

@hjacobs
Copy link
Owner

hjacobs commented Mar 28, 2020

Thanks, but this also removes the normpath call which IMHO should be added back.

@jbw
Copy link
Author

jbw commented Mar 28, 2020

normpath

@hjacobs thanks for picking this project up by the way.

The problem with normpath is is it meant for file system interactivity and will handle separators depending on the OS. Using normpath here will introduce the same problem.

I guess the larger problem here is that URLs are being created with OS path modules. I'm not close with the code base so, what benefit does normpath have in this part of code for constructing URLs?

@hjacobs
Copy link
Owner

hjacobs commented Mar 29, 2020

I just tested it --- it won't work without normpath! I'm now adding a function to do the URL join without filesystem/posix functions.

@hjacobs
Copy link
Owner

hjacobs commented Mar 29, 2020

I implemented a new utility function to stop using filesystem path functions: #59

@hjacobs hjacobs closed this Mar 29, 2020
@jbw jbw deleted the use-posixpath-for-url branch April 1, 2020 16:09
@jbw
Copy link
Author

jbw commented Apr 1, 2020

I implemented a new utility function to stop using filesystem path functions: #59

Thanks! It is a shame this won't solve my problem in the original pykube repo which i currently depend on!

@hjacobs
Copy link
Owner

hjacobs commented Apr 1, 2020

@jbw what makes you still depend on the original pykube repo (https://github.com/kelproject/pykube)?

@jbw
Copy link
Author

jbw commented Apr 1, 2020

@hjacobs i'm using Luigi (https://github.com/spotify/luigi) and it's Kubernetes integration (https://github.com/spotify/luigi/blob/master/luigi/contrib/kubernetes.py). It should be the case i can simply substitute pykube for pykube-ng (assuming the api is the same). I'm going to test this out and see.

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

Successfully merging this pull request may close these issues.

2 participants