Update QA form to handle the individual checks.

Update QA form to handle the individual checks.

- Add the new migration for loading the question data.
- Update the form schema to dynamically generate/add the scoring checks
- Make sure the form is rendered with the proper bootstrap templates
(mapping.pt, radio_choice.pt)
- Lint fixes for makeUser
- Update the QAAssessment factory method to generate the new schema for POST
- Add a factory method to add the db.qa collection so questions are processed.
- Update the qa form tests to check the new schema.
- Update the ingest script to know where it's located and find all paths
relative to itself. All deployments (even charm) are in a venv so safe.
- Screenshot of the updated form rendering with the grouped radio buttons:
http://uploads.mitechie.com/lp/charmworld_radio_controls.png

https://code.launchpad.net/~rharding/charmworld/qa_assessment_form2/+merge/142515

(do not edit description out of merge proposal)

[rharding, 2013-01-09 15:09:46]

Please take a look.

[rharding, 2013-01-09 15:11:35]

Please take a look.

[rharding, 2013-01-09 15:16:24]

Please take a look.

[rharding, 2013-01-09 16:10:40]

Please take a look.

[hazmat, 2013-01-09 19:41:12]

needs work

https://codereview.appspot.com/7066059/diff/13/charmworld/forms/qa_assessment.py
File charmworld/forms/qa_assessment.py (right):

https://codereview.appspot.com/7066059/diff/13/charmworld/forms/qa_assessment...
charmworld/forms/qa_assessment.py:114: ('1', 'Yes'),
seems a little strange to have stringified ints as values, its not clear that
its relevant enough to change even with indexing given the storage.

https://codereview.appspot.com/7066059/diff/13/charmworld/views/charms.py
File charmworld/views/charms.py (right):

https://codereview.appspot.com/7066059/diff/13/charmworld/views/charms.py#new...
charmworld/views/charms.py:342: qa_questions = getdb().qa.find()
we'll need some query discriminator in the future for atomic switch out of
working set in the future.. but yagni for now.

https://codereview.appspot.com/7066059/diff/13/scripts/ingest
File scripts/ingest (right):

https://codereview.appspot.com/7066059/diff/13/scripts/ingest#newcode42
scripts/ingest:42: $PYTHON_BIN $SCRIPT_DIR/load_qaassessment_data.py
this shouldn't be here.. the rest of this script is re-entrant and is executed
multiple times, the qa form is a one time db initialization its not part of the
ingest pipeline, ie. if you run this script twice, you'd end up with duplicate
questions.

https://codereview.appspot.com/7066059/diff/13/scripts/load_qaassessment_data.py
File scripts/load_qaassessment_data.py (right):

https://codereview.appspot.com/7066059/diff/13/scripts/load_qaassessment_data...
scripts/load_qaassessment_data.py:121: 'id': hashlib.md5(q[0]).hexdigest(),
i think this should be an explicit unique label instead of the hash, which is
subject to change if we reword something which would result in answer mismatch
though they are materially the same, and it also allows for natural querying
from the charm collection without dereferencing via the qa form collection.

[rharding, 2013-01-09 20:07:06]

Thanks, I'll go back at things. Appreciate the time for the review.

https://codereview.appspot.com/7066059/diff/13/charmworld/forms/qa_assessment.py
File charmworld/forms/qa_assessment.py (right):

https://codereview.appspot.com/7066059/diff/13/charmworld/forms/qa_assessment...
charmworld/forms/qa_assessment.py:114: ('1', 'Yes'),
On 2013/01/09 19:41:12, hazmat wrote:
> seems a little strange to have stringified ints as values, its not clear that
> its relevant enough to change even with indexing given the storage.

The field is defined as a String because I need three values since there's the
'unknown' value. Anything that's an int would potentially mess up the math of
adding the scores. This way it's hopefully more clear that you can't just
sum([field for field in question__list]).

https://codereview.appspot.com/7066059/diff/13/scripts/ingest
File scripts/ingest (right):

https://codereview.appspot.com/7066059/diff/13/scripts/ingest#newcode42
scripts/ingest:42: $PYTHON_BIN $SCRIPT_DIR/load_qaassessment_data.py
On 2013/01/09 19:41:12, hazmat wrote:
> this shouldn't be here.. the rest of this script is re-entrant and is executed
> multiple times, the qa form is a one time db initialization its not part of
the
> ingest pipeline, ie. if you run this script twice, you'd end up with duplicate
> questions.

Currently there's no good place to load the data. My main concern is that the
data is loaded on all instances without having to manually instruct everyone to
run a script. Once the data format is done and we know what we're doing, the
plan is to move it to the migrations work that's next in line to be done. I
suppose I can put this branch on hold and work on migrations, but my hope was to
get this solid and split up the rest of the work around the qa data among
several developers.

You're right on the dupe data though. It was hidden by the schema generation. I
had thought it was safe as it'd replace itself in place, but the _id would be
new. I'll rework this.

https://codereview.appspot.com/7066059/diff/13/scripts/load_qaassessment_data.py
File scripts/load_qaassessment_data.py (right):

https://codereview.appspot.com/7066059/diff/13/scripts/load_qaassessment_data...
scripts/load_qaassessment_data.py:121: 'id': hashlib.md5(q[0]).hexdigest(),
On 2013/01/09 19:41:12, hazmat wrote:
> i think this should be an explicit unique label instead of the hash, which is
> subject to change if we reword something which would result in answer mismatch
> though they are materially the same, and it also allows for natural querying
> from the charm collection without dereferencing via the qa form collection.

Well, the original idea was I just need a pk/unique id. Keeping this as
something manually done would be challenging. This just generates a key for the
initial data load, but changes to the wording only effects if the data is
reloaded which it won't be. The original plan is that when new questions are
added they'd get a random key generated as well. That key never changes for that
question as long as it lives.

[hazmat, 2013-01-10 04:08:25]

On 2013/01/09 20:07:06, rharding wrote:
> Thanks, I'll go back at things. Appreciate the time for the review.
> 
>
https://codereview.appspot.com/7066059/diff/13/charmworld/forms/qa_assessment.py
> File charmworld/forms/qa_assessment.py (right):
> 
>
https://codereview.appspot.com/7066059/diff/13/charmworld/forms/qa_assessment...
> charmworld/forms/qa_assessment.py:114: ('1', 'Yes'),
> On 2013/01/09 19:41:12, hazmat wrote:
> > seems a little strange to have stringified ints as values, its not clear
that
> > its relevant enough to change even with indexing given the storage.
> 
> The field is defined as a String because I need three values since there's the
> 'unknown' value. Anything that's an int would potentially mess up the math of
> adding the scores. This way it's hopefully more clear that you can't just
> sum([field for field in question__list]).


does the deform scheme node respect None as a null value? if so then you have
None, 0, 1 which sum quite nicely in conjunction with mongo's aggregate
functions against the collection of charms.


> 
> https://codereview.appspot.com/7066059/diff/13/scripts/ingest
> File scripts/ingest (right):
> 
> https://codereview.appspot.com/7066059/diff/13/scripts/ingest#newcode42
> scripts/ingest:42: $PYTHON_BIN $SCRIPT_DIR/load_qaassessment_data.py
> On 2013/01/09 19:41:12, hazmat wrote:
> > this shouldn't be here.. the rest of this script is re-entrant and is
executed
> > multiple times, the qa form is a one time db initialization its not part of
> the
> > ingest pipeline, ie. if you run this script twice, you'd end up with
duplicate
> > questions.
> 
> Currently there's no good place to load the data. My main concern is that the
> data is loaded on all instances without having to manually instruct everyone
to
> run a script. Once the data format is done and we know what we're doing, the
> plan is to move it to the migrations work that's next in line to be done. I
> suppose I can put this branch on hold and work on migrations, but my hope was
to
> get this solid and split up the rest of the work around the qa data among
> several developers.


there's an initialize_db being imported from models that would seem to be the
logical place for database init/one time loading.


> 
> You're right on the dupe data though. It was hidden by the schema generation.
I
> had thought it was safe as it'd replace itself in place, but the _id would be
> new. I'll rework this.
> 


just querying the collection to check empty would suffice, and let migrations
take care of loading against non-null.


>
https://codereview.appspot.com/7066059/diff/13/scripts/load_qaassessment_data.py
> File scripts/load_qaassessment_data.py (right):
> 
>
https://codereview.appspot.com/7066059/diff/13/scripts/load_qaassessment_data...
> scripts/load_qaassessment_data.py:121: 'id': hashlib.md5(q[0]).hexdigest(),
> On 2013/01/09 19:41:12, hazmat wrote:
> > i think this should be an explicit unique label instead of the hash, which
is
> > subject to change if we reword something which would result in answer
mismatch
> > though they are materially the same, and it also allows for natural querying
> > from the charm collection without dereferencing via the qa form collection.
> 
> Well, the original idea was I just need a pk/unique id. Keeping this as
> something manually done would be challenging. This just generates a key for
the
> initial data load, but changes to the wording only effects if the data is
> reloaded which it won't be. The original plan is that when new questions are
> added they'd get a random key generated as well. That key never changes for
that
> question as long as it lives.




That doesn't resolve the natural query capability that document dbs naturally
lend themselves to with human understandable keys. imo, its quite ugly in a
document oriented db to have indecipherable keys that need a join to deciper.
ie. its much nicer to query on db.charms.query({'qa.reliable.ec2': 1}) then 
using {'qa.reliable.34e37456112085f7b71b6347f8184a73': 1}

its effectively just a django url slug field. if the hash isn't a hash, then
just making it random seems more appropriate, else its easy to generate
collisions. in either case (unique human key or rand) a unique index is going to
be nesc (along with the appropriate driver flag to detect errors, on by default
in latest driver releases).

[rharding, 2013-01-11 18:04:15]

Please take a look.

[rharding, 2013-01-11 18:16:29]

On 2013/01/10 04:08:25, hazmat wrote:

> does the deform scheme node respect None as a null value? if so then you have
> None, 0, 1 which sum quite nicely in conjunction with mongo's aggregate
> functions against the collection of charms.

The colander package has a special colander.null. However, I ran into issue
trying to set that as one of the choices for the form representation. I did get
it to work as a default value, but then mongo can't serialize colander.null into
the database. 

While using a string seems a little dirty, I think it works best currently
across the three tools, colander, deform, and pymongo without manually monkeying
with the data along the process which would complicate loading/storing form
results more.


> just querying the collection to check empty would suffice, and let migrations
> take care of loading against non-null.

Updated to use the new migrations tooling to load values.

> That doesn't resolve the natural query capability that document dbs naturally
> lend themselves to with human understandable keys. imo, its quite ugly in a
> document oriented db to have indecipherable keys that need a join to deciper.
> ie. its much nicer to query on db.charms.query({'qa.reliable.ec2': 1}) then 
> using {'qa.reliable.34e37456112085f7b71b6347f8184a73': 1}
> 
> its effectively just a django url slug field. if the hash isn't a hash, then
> just making it random seems more appropriate, else its easy to generate
> collisions. in either case (unique human key or rand) a unique index is going
to
> be nesc (along with the appropriate driver flag to detect errors, on by
default
> in latest driver releases).

So, I've moved half way on this. I renamed the questions $category.$number. So
that it's a bit readable. However, I feel that any UI for doing these searches
would present a list of fields/options from the list of questions and the id
would be known. 

With the new id, say secure.2. It's easy for us to do that search using:
db.charms.find({"qa.secure.secure_3": "1"})

[rharding, 2013-01-11 18:17:39]

Please take a look.

[hazmat, 2013-01-11 20:19:51]

On Fri, Jan 11, 2013 at 12:16 PM, <rharding@mitechie.com> wrote:

> On 2013/01/10 04:08:25, hazmat wrote:
>
>  does the deform scheme node respect None as a null value? if so then
>>
> you have
>
>> None, 0, 1 which sum quite nicely in conjunction with mongo's
>>
> aggregate
>
>> functions against the collection of charms.
>>
>
> The colander package has a special colander.null. However, I ran into
> issue trying to set that as one of the choices for the form
> representation. I did get it to work as a default value, but then mongo
> can't serialize colander.null into the database.
>
> While using a string seems a little dirty, I think it works best
> currently across the three tools, colander, deform, and pymongo without
> manually monkeying with the data along the process which would
> complicate loading/storing form results more.



sounds good, thanks for looking.


>
>
>  just querying the collection to check empty would suffice, and let
>>
> migrations
>
>> take care of loading against non-null.
>>
>
> Updated to use the new migrations tooling to load values.
>
>
>  That doesn't resolve the natural query capability that document dbs
>>
> naturally
>
>> lend themselves to with human understandable keys. imo, its quite ugly
>>
> in a
>
>> document oriented db to have indecipherable keys that need a join to
>>
> deciper.
>
>> ie. its much nicer to query on db.charms.query({'qa.reliable.**ec2': 1})
>>
> then
>
>> using {'qa.reliable.**34e37456112085f7b71b6347f8184a**73': 1}
>>
>
>  its effectively just a django url slug field. if the hash isn't a
>>
> hash, then
>
>> just making it random seems more appropriate, else its easy to
>>
> generate
>
>> collisions. in either case (unique human key or rand) a unique index
>>
> is going to
>
>> be nesc (along with the appropriate driver flag to detect errors, on
>>
> by default
>
>> in latest driver releases).
>>
>
> So, I've moved half way on this. I renamed the questions
> $category.$number. So that it's a bit readable. However, I feel that any
> UI for doing these searches would present a list of fields/options from
> the list of questions and the id would be known.
>
> With the new id, say secure.2. It's easy for us to do that search using:
> db.charms.find({"qa.secure.**secure_3": "1"})
>

not sure that half-way gets us anywhere different, it still needs a lookup
table to resolve. what's the issue with the short key per question?

thanks,

kapil

[rharding, 2013-01-11 21:46:38]

On 2013/01/11 20:19:51, hazmat wrote:
> >
> > So, I've moved half way on this. I renamed the questions
> > $category.$number. So that it's a bit readable. However, I feel that any
> > UI for doing these searches would present a list of fields/options from
> > the list of questions and the id would be known.
> >
> > With the new id, say secure.2. It's easy for us to do that search using:
> > db.charms.find({"qa.secure.**secure_3": "1"})
> >
> 
> not sure that half-way gets us anywhere different, it still needs a lookup
> table to resolve. what's the issue with the short key per question?
> 
> thanks,
> 
> kapil

The problem with the short key is that it's vague. What's short, and as a key,
it's completely up to the person that enters it. As this is something that we
eventually want to enable users to add/remove questions via the webui, I really
hate putting the key into user hands like that. 

In this way, it's searchable, short, and 'makes sense' when looked at. Honestly,
the data is in the db and exposed via the api/ui and the only reason a 'words'
short string id would help is for use cases where users would be on the fly
doing queries by hand into the db itself. 

The short key still needs a lookup table to solve because you have to know what
the inserter of the record thought a 'short key' that made sense would be. 

Maybe I'm misunderstanding the use case. What's the user story/UX that we're
looking to solve/