projectatomic / atomicapp

[UNMAINTAINED] This is the reference implementation of the Nulecule container application Specification: Atomic App
102 stars 71 forks source link

Empty fields accepted in Atomic App #716

Open surajssd opened 8 years ago

surajssd commented 8 years ago

Is it safe/correct behavior to accept empty fields? AtomicApp accetps empty fields:

$ sudo atomicapp run . -v 
1461836612 - [INFO] - cli/main.py - Action/Mode Selected is: run
1461836612 - [DEBUG] - cli/main.py - Final parsed cmdline: run . -v
[SNIP]
1461836612 - [DEBUG] - atomicapp/plugin.py - Loading providers from /home/vagrant/atomicapp/atomicapp/providers
==> db_user (Database User): 
==> db_pass (Database Password): 
==> db_name (Database Name): 
==> db_user (Database User): 
==> db_pass (Database Password): 
==> db_name (Database Name): 
1461836615 - [DEBUG] - atomicapp/plugin.py - Found provider <class 'kubernetes.KubernetesProvider'>
1461836615 - [WARNING] - atomicapp/plugin.py - Configuration option 'provider-config' not found
[SNIP]
1461836615 - [DEBUG] - nulecule/main.py - Writing answers to file.
1461836615 - [DEBUG] - nulecule/main.py - FILE: ./answers.conf.gen
1461836615 - [DEBUG] - nulecule/main.py - ANSWERS: {u'mariadb-centos7-atomicapp': {u'provider': u'kubernetes'}, u'etherpad-app': {u'image': u'centos/etherpad', u'db_pass': '', u'db_name': '', u'db_user': '', u'db_host': u'mariadb', u'hostport': 9001}, u'mariadb-atomicapp': {u'db_pass': '', u'db_name': '', u'db_user': '', u'root_pass': u'MySQLPass'}, 'general': {'namespace': 'default', u'provider': u'kubernetes'}}

Your application resides in .
Please use this directory for managing your application

Generated answers file

$ cat answers.conf.gen 
[mariadb-centos7-atomicapp]
provider = kubernetes
[etherpad-app]
image = centos/etherpad
db_pass = ""
db_name = ""
db_user = ""
db_host = mariadb
hostport = 9001
[mariadb-atomicapp]
db_pass = ""
db_name = ""
db_user = ""
root_pass = MySQLPass
[general]
namespace = default
provider = kubernetes

Also saw the pod situation after some time:

$ kubectl get pods
NAME             READY     STATUS             RESTARTS   AGE
etherpad-n77jl   0/1       CrashLoopBackOff   8          19m
mariadb          1/1       Running            0          19m
surajssd commented 8 years ago

This is etherpad-centos7-atomicapp. But not sure the above CrashLoopBackOff is due to empty fields.

cdrage commented 8 years ago

I've experienced this recently as well...

In regards to the .gen file. Should all of this be None? @dustymabe

cdrage commented 8 years ago

Could possibly be beginner friendly (may be a bit of a challenge :))

dustymabe commented 8 years ago

So I think we probably shouldn't accept empty fields we should probably query the user again if they don't provide an answer.

Is there a case where not providing an answer is valid? I know if there is a default then not providing an answer is acceptable because the default will be used.

concaf commented 8 years ago

Is there a case where not providing an answer is valid?

@dustymabe @surajssd @cdrage - There could be cases when the fields such as db_pass for some dev environments could be blank.

Should this not be left up to the end user, than atomicapp having an opinion about whether the field should be blank or not?

dustymabe commented 8 years ago

@dustymabe @surajssd @cdrage - There could be cases when the fields such as db_pass for some dev environments could be blank.

@containscafeine when you say blank, do you mean the empty string or do you literally mean no password?

concaf commented 8 years ago

@dustymabe well, no password.

dustymabe commented 8 years ago

In that case maybe what we should do is prompt the user and say "you have not given an answer for the question, are you sure you want to leave it blank?"

concaf commented 8 years ago

@dustymabe or prefix the prompt with Enter db_pass (leave empty for no value):, so we don't have to prompt twice.

dustymabe commented 8 years ago

ehh, i think i'd rather not affect the 90% use case for the 10% corner case. @cdrage what do you think about this conversation?

cdrage commented 8 years ago

Imo, by default we should have it as a blank string rather than None in the Nulecule configuration file. Although we should still prompt the user for input on the string. For example, if we prompt up asking for db_dev_test have Atomic App disclaim that it was blank ex. db_dev_test (Default: "") ==>

dustymabe commented 8 years ago

Imo, by default we should have it as a blank string rather than None in the Nulecule configuration file.

You mean in the answers.conf file or in the Nulecule?