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).