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

Support -f url to specify config url #3098

Closed
wants to merge 1 commit into from
Closed

Conversation

yongjhih
Copy link

@yongjhih yongjhih commented Mar 9, 2016

For example:

docker-compose -f https://github.com/yongjhih/docker-parse-server/raw/master/docker-compose.yml up

@@ -213,6 +214,9 @@ def find(base_dir, filenames):
os.getcwd(),
[ConfigFile(None, yaml.safe_load(sys.stdin))])

if filename.startswith('./http'):

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

How about protocol testing for urllib supported?
I think we could support usual protocols only.

Choose a reason for hiding this comment

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

I think ./http is kind of hack. But @sergeyklay were you talking about supporting all the protocols? Or you're saying you prefer to support stdin and local file system only?

Choose a reason for hiding this comment

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

I mean support of usual protocols, like ftp, http, etc.

Choose a reason for hiding this comment

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

Actually, reference to the build system of Go or Java, I think we can simply support HTTP/HTTPS. Because they're much more popular them other protocols. curl http://stuff/to/download | docker-compose -f - is just a little bit long. And you know what, the container version of docker-compose cannot even read stdin properly. I think that's a bug but this can be a reason for us to consider the improvement.

@yongjhih yongjhih force-pushed the config branch 4 times, most recently from 06e0cb6 to de30c6d Compare March 9, 2016 14:49
@dnephin
Copy link

dnephin commented Mar 9, 2016

Related to #1818 (comment)

I like the idea, but I don't think we should overload -f. I think it would be better to make a new flag --url because downloading a remote file can fail in many more ways than just reading a local file.

with open(filename, 'r') as fh:
return yaml.safe_load(fh)
if is_compose_url(filename):
return yaml.safe_load(urllib.urlopen(filename))
Copy link

Choose a reason for hiding this comment

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

This is going to need a lot more error handling. There are a lot of failures that can happen here.

Copy link
Author

Choose a reason for hiding this comment

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

Allow error handling by response.raise_for_status()

@sergeyklay
Copy link

+1 for --url

@pimterry
Copy link

I'd really like to use this, but it looks like the builds are broken right now, and just by minor things: a too long line in the tests and incorrectly ordered imports. @yongjhih, any chance you could fix those up soon?

@yongjhih
Copy link
Author

yongjhih commented Apr 1, 2016

I used to vim http://xxx not vim --url http://xxx, so i don's think make a new --url flag.

@yongjhih yongjhih force-pushed the config branch 9 times, most recently from ca6b0aa to 798a33f Compare April 1, 2016 10:18
@yongjhih yongjhih force-pushed the config branch 3 times, most recently from 5773d1b to 6f9b828 Compare April 1, 2016 10:57
@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "config" git@github.com:yongjhih/docker-compose.git somewhere
$ cd somewhere
$ git rebase -i HEAD~2
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Ammending updates the existing PR. You DO NOT need to open a new one.

2 similar comments
@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "config" git@github.com:yongjhih/docker-compose.git somewhere
$ cd somewhere
$ git rebase -i HEAD~2
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Ammending updates the existing PR. You DO NOT need to open a new one.

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "config" git@github.com:yongjhih/docker-compose.git somewhere
$ cd somewhere
$ git rebase -i HEAD~2
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Ammending updates the existing PR. You DO NOT need to open a new one.

For example:

    docker-compose -f https://github.com/yongjhih/docker-parse-server/raw/master/docker-compose.yml up

Signed-off-by: Andrew Chen <yongjhih@gmail.com>
@denismakogon
Copy link

+1 to previous reviews, --url should be the best option here, so in general, it looks good, but please consider to use different flag.

@yongjhih
Copy link
Author

yongjhih commented May 24, 2016

@pimterry
Fixed, all checks have passed.

@shin- shin- closed this Mar 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants