Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor update architecture to "plug in" #147

Merged
merged 6 commits into from
Oct 27, 2014

Conversation

patt0
Copy link
Collaborator

@patt0 patt0 commented Oct 27, 2014

  1. Expresses an architecture to gather new posts and update them using app engine tasks, which are scheduled using cron jobs. This same approach will be used to write data gathering plugins so we can collect impact metrics automatically. I intend to get one ready that pulls view data from blogger in time for the presentation at the GDE summit so we may invite many GDE's to write data gathering plugins (I use the word plugin loosely).
  2. /tasks/new_gplpus is run daily and as the previous cron job gets new activities, this time for one gde at the time.

The implementation is different in that we only merge AP is they are actual shares from another post, if they are not we will create a new AR and leave it to the GDE to decide how and why he merges AR's and their associated AP's. In order to do this, I process the new activities from oldest to newest.

  1. /tasks/upd_gplus is run weekly and updates existing activities, similarly as above for one gde at the time.

The implementation is pretty much the same as previously. In particular we continue to ignore the fact that a #gde tag is removed from a gplus post, we consider currently that it has to be deleted in the front end, to be discussed at the summit.

  1. ActivityRecord -> find_or_create as a function, is really the responsibility of the Business Logic (ie. the task plugin architecture) so I am moving it out of ActivityPost Endpoints class and attaching it to update_gplus Module in the name find_or_create_ar.
  2. The API_KEY to access the GPLUS service is now obtained from secrets.json and the keys have been regenerated, so they are not what they where it the source code tree anymore.
  3. The insert ActivityPost endpoint has been removed.

@Scarygami thanks for having a good look at this :)

user_count = 0
for account in accounts:
#don't process admin users
if account.type == "administrator":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we had plans to keep "inactive" GDEs in the Account list, but I think those shouldn't be included in the update task either, so maybe make the check:

if account.type != "active":

@Scarygami
Copy link
Contributor

Except for the two minor non-critical inline-comments I made this looks great! 👍

Thanks!

user_count = 0
for account in accounts:
#don't process admin users
if account.type == "administrator":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing as above:

if account.type != "active":

@patt0
Copy link
Collaborator Author

patt0 commented Oct 27, 2014

#54 can be closed

@Scarygami
Copy link
Contributor

Sounds/looks good to me, feel free to merge and close whenever you deploy this into production :)

patt0 added a commit that referenced this pull request Oct 27, 2014
refactor update architecture to "plug in"
@patt0 patt0 merged commit 6b1f843 into crolimax:feature/appengine Oct 27, 2014
@patt0
Copy link
Collaborator Author

patt0 commented Oct 27, 2014

Disabling writes, taking a backup and uploading the code, reenable writes -> All Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants