-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111654/#review36379
-----------------------------------------------------------


why a clone of the discourse repo on github? was this to ensure that changes 
that happen in upstream discourse do not cause unexpected problems? or do you 
have actual changes in your github clone?

the installation procedure needs to be recorded in the README, but i suppose i 
can do that once this is in.

one this patch is in, there will need to modifications made to the asset info 
API in bodega-server to include a link to the discussion forum. that implies 
adding a new service block to the config.json for things like the url to the 
discourse server. 


sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26869>

    this line should go into the sql/superuserCommand.sql file. this can happen 
post-merge, however.



sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26862>

    the name of the database needs to come from configuration somewhere; as we 
can't use the config.json from the nodejs app, it will need to be stored in the 
database.
    
    i need this for other things as well, so will take care of this once it is 
committed.



sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26863>

    there should be an EXCEPTION block here to catch the case where the dblink 
fails.
    
    that way the exception can be dealt with and it won't disrupt any 
transactions that might be happening otherwise.
    
    right now, if the dblink fails to the discourse system, then updating the 
partner data will likely just fail.



sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26864>

    all new partners added will start at 1000, as the seq_partnerId sequence is 
set to 1000 in defaultData.sql. so this is not an issue, and this block should 
be commented back in.
    
    this check should also be moved to the very start of the method since none 
of this should run for such "system" partners.



sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26870>

    i believe there is a race condition here due to dblink being async between 
2 databases (so the usual "a function is automatically run in a transaction" 
safety does not apply here).
    
    instead, add a "RETURNING id" to the "INSERT INTO categories.." query and 
fetch it from there.
    
    same thing with the topic insert below.



sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26871>

    the names are not guaranteed to be unique between assets.
    
    if two people have an asset named "Awesome" this will cause problems.
    
    i would recommend storing the category and topic id's from the discourse db 
in bodega db or alternatively (and this would probably be nicer) add a reliable 
id to the topics/categories in the the discourse db. (e.g. based on the asset's 
ID, which never changes)
    
    this could be done non-intrusively by adding another table to the discourse 
db where these "bodega" ids are stored, perhaps?



sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26865>

    an exception block is also required here for the same reason.


- Aaron J. Seigo


On July 23, 2013, 10:20 a.m., Giorgos Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111654/
> -----------------------------------------------------------
> 
> (Updated July 23, 2013, 10:20 a.m.)
> 
> 
> Review request for Bodega.
> 
> 
> Description
> -------
> 
> This patch keeps in sync the DBs of the bodega-server and discourse.
> 
> It syncs,
> 
> a. the user accounts*
> b. for each asset that has a partner with id > 1000 it create a new forum 
> which has a topic and a post
> 
> * there is a default account for the discourse
> username: forumadmin
> password: password
> 
> 
> Diffs
> -----
> 
>   server/bodegaDbHelper 273e34f 
>   sql/discourse.plsql PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/111654/diff/
> 
> 
> Testing
> -------
> 
> How to test it,
> 
> * git clone https://github.com/terietor/discourse.git
> * cd discourse
> * git checkout bodega
> * https://github.com/terietor/discourse/blob/master/docs/DEVELOPER-ADVANCED.md
> * cd bodega-server/server
> * make sql && make test-sql (your bodega user must be a superuser in order to 
> enable the dblink extention)
> * then go back to discourse and do `bundle exec rails s`
> * go to localhost:3000
> 
> 
> Thanks,
> 
> Giorgos Tsiapaliokas
> 
>

_______________________________________________
Active mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/active

Reply via email to