metaflow
metaflow copied to clipboard
Netflix
fix: Removed forward slash to prevent double slashes
I made a slight change to this. Let me know what you think but it LGTM otherwise.
One note on this though: this will be a breaking change of sorts in the sense that it may point things to a different "bucket".
I made a slight change to this. Let me know what you think but it LGTM otherwise.
Thank You. rstrip solution is better than removing "/". Also if in future someone thinks of removing "/" directly from config then this change will not break.
One note on this though: this will be a breaking change of sorts in the sense that it may point things to a different "bucket".
Need to check this. Can u point me in a direction where i can test this?
One note on this though: this will be a breaking change of sorts in the sense that it may point things to a different "bucket".
Need to check this. Can u point me in a direction where i can test this?
Well, I am just saying that if your config had a / at the end, you will be pointing to a bucket like something//foo and now it would be something/foo which isn't the same in S3.
@vamagithub Given that this change is going to be backwards incompatible, rather than making the change here, what do you think of making the change in the main_cli.py instead to ensure that any new configs that are generated, don't have any extraneous slashes?
@vamagithub Given that this change is going to be backwards incompatible, rather than making the change here, what do you think of making the change in the
main_cli.pyinstead to ensure that any new configs that are generated, don't have any extraneous slashes?
I agree, that would be better. Let me know what can i do to help.
Would it be cleaner to leverage the os.path (or even s3path) module to cleanly get consistent paths?
Going through older PR's, is this one still active and relevant? After a quick test I was at least unable to reproduce the original issue with trailing slashes in the config
It is probably still valid but would be a breaking change since "foo//bar" is different than "foo/bar" in S3. It tries to "do the right thing by the user" if the user has a trailing slash in their config.