Allow links to containers outside of the project

[jbalonso]

Resolves #159. I believe this PR needs tests (existing ones should be ok), but I wanted to start a conversation around it.

[dnephin]

I think this would be cool. I don't really like the :existing_container_name syntax. I don't think it's really obvious what's going on.

I hope we can come up with a better way to represent "external". Some ideas:

• a symbol that isn't valid for a container name? <external_container_name or +external_container_name
• maybe a new keyword? external=external_container_name
• a new config key

    docker_links:
        - external_container_name

[jbalonso]

sigh I did implement it as a new config keyword originally (external_links), but I dropped those commits in favor of the #159 format when I found that issue--I was uncomfortable breaking the pattern of docker command line keywords being fig.yml keywords, even though adding a keyword involved less invasive code. I do regret, however, not being able to rename external containers readily (that is, existing_container_name:link_name format).

I'll see if my repo has managed not to garbage collect my older commits.

[jbalonso]

I managed to dig up my implementation in the form of an external_links config keyword. It's now the form of this PR.

[jbalonso]

grumble this build failed for external reasons. The flake8 error is gone in my local tests, but I suppose I'll let this sit here until we can devise some real tests.

[jbalonso]

I believe this would also resolve #428, for the record.

Automated tests are still forthcoming. My team is a bit busy over here.

[cgcgbcbc]

The test failed for Failed to fetch http://http.debian.net/debian/pool/main/p/python2.6/python2.6_2.6.8-1.1_amd64.deb Error reading from server. Remote end closed connection [IP: 46.4.205.44 80] Fetched 103 MB in 13s (7559 kB/s)...

[jbalonso]

Yep. See "failed for external reasons." :) I expect it would be cleared up when we push out another commit.

If someone wants to PR a test into our branch at LuminosoInsight/fig, we could hurry this PR along. Otherwise... my team is tied up for a while.

[cgcgbcbc]

I expect it would be cleared up when we push out another commit.

You can use git rebase to produce a "new" commit and force update the branch.

# on branch external-links
$ git rebase -i HEAD^
# then replace pick with e
$ git commit --amend
# if you use vim as commit editor, simply delete the last letter of the commit message and readd id, then save
$ git rebase --continue # then it will produce a commit which is the same with current HEAD, but different sha1.
$ git push -f # assuming that your push.default is simple.

[mauricioabreu]

Is this PR still worth? I am willing to write tests if needed.

[jbalonso]

I think it's still worthwhile (but I'm biased)--my team is actively using and fundamentally requires our branch.

[mauricioabreu]

I need this feature as well. Like you we are using an isolated branch.

[roytruelove]

+1 definitely necessary; I have a single instance of a database that's shared by multiple fig-managed containers

[jbalonso]

The +1's are all well and good, but if someone could contribute a test to our branch, this PR could move along. (My team is still tied up in other matters.)

[andrewmichaelsmith]

@jbalonso Has there been any more feedback from the fig guys about if this is something they want?

[mauricioabreu]

@jbalonso I just created a pull request against your branch.
Let me know it that is enough :-)

[mauricioabreu]

Sorry, I forgot to link my PR against @jbalonso branch: https://github.com/LuminosoInsight/fig/pull/2

[dnephin]

Cool, I think we just need an entry in the docs to show how this would work

[mauricioabreu]

hey @dnephin I added another PR here: https://github.com/LuminosoInsight/fig/pull/3

Just waiting for feedback and merge so we can discuss the docs details here.

[jbalonso]

I've merged @mauricioabreu's PR and touched it up slightly.

[jbalonso]

grumble Fetch failure... I'll try again with a squash-and-force-push.

[jbalonso]

shakes fist. Ok... it seems that the build system is in a bad mood today, so I'll try again later, maybe tomorrow, unless someone here says that I can try again and reasonably expect something different.

[mauricioabreu]

so sad :(

[cerisier]

+ 1

[suaron]

👍

[mauricioabreu]

hey @jbalonso can you try another push? 😭

[jbalonso]

The tests have passed.

[thaJeztah]

🎉

[mauricioabreu]

Thank you all for all your efforts.

[mauricioabreu]

I think we need a rebase (pick/squash) here.

[jbalonso]

Why is that, @mauricioabreu ?

[mauricioabreu]

Because most of the projects don't want a lot of commits. I don't know how fig works but they probably will want a rebase (pick/squash) here.

[aanand]

Squash and rebase, yes please - plus the docs seem to say external-links when it should be external_links.

[mauricioabreu]

Right @aanand
I think @jbalonso can modify this typo in the docs at the time he rebases.

[jbalonso]

Mischief managed. Here's to hoping that the tests pass.

[jbalonso]

And they passed! 🎉 (Happy Almost New Year!)

[mauricioabreu]

I think it is ok now! Thanks.

[aanand]

LGTM

[tsileo]

Any update on this ?

[dnephin]

LGTM

[thaJeztah]

Oh yeah!