skybet / cali

Cali Automation Layout Initialiser
MIT License
32 stars 7 forks source link

Add optional name for tasks and docker containers #22

Closed wheresalice closed 6 years ago

wheresalice commented 6 years ago

I'm pretty new to both working with Go and working on this codebase, but this works for me as a way of optionally setting a container name as referenced in issue #4

Usage:

    task := command.Task("shaynativ/redis-ml")
    task.Name = "bob"

Any advice on better ways of doing it would be appreciated.

wheresalice commented 6 years ago

This doesn't cover the requirements in #4 but it gives me enough to do what I need to do for datali.

lucymhdavies commented 6 years ago

This would work under some circumstances.

However if both of the Task's TaskFuncs are docker image names, you'd end up with both of them running with the same name. This isn't necessarily an issue, as both would not exist at the same time.

It would however mean that you cannot run two instances of your Task at the same time though, unless your app was very careful about ensuring that t.Name was unique.

I'll have a think.

lucymhdavies commented 6 years ago

Ah. I think I've figured out why this doesn't feel right.

It's the assumption of a 1-to-1 mapping of Task to Docker container, which is sorta an assumed thing, and basically how most Cali Commands would work, but technically you can have two Docker containers in your Task. And perhaps it should be more like a Command can have multiple Tasks, rather than 1 Command = 1 Task = 2 TaskFuncs.

I need to think this through, and make some breaking changes to Cali at some point, when I figure out how I think this should work.