-
Notifications
You must be signed in to change notification settings - Fork 14
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
Event Importer (Meetup + Eventbrite) along with Events Api + Org Api #186
Event Importer (Meetup + Eventbrite) along with Events Api + Org Api #186
Conversation
…dd-env-example Fix cast of service_id and add .env.example for EventBrite API token
@ThorntonMatthewD thank you! |
Is it worth it to add in support for the following query strings to the org endpoint? https://github.com/hackgvl/OpenData/blob/master/ORGANIZATIONS_API.md#query-string I'm not sure if this is meant to be a replacement or a passthrough. |
I posted related thoughts on the Org side on #187 (comment) @bogdankharchenko sorry it's taken me so long to give this attention. I wanted to give it a my full attention before commenting, but then went from traveling, to being sick while trying to catch back up from traveling. I haven't done a technical review, and trust I won't be of much use there anyway, but my main thought was we could test out the new API endpoint using less critical consuming apps to start, like the OpenWorks dashboard and the Slack Bot. If there's something that breaks with those it would likely be more obvious, easier to fix or revert, and impacting a smaller audience. Also, I wonder if we'll have some of the same challenges with dev copies of this when there's a database component where we want more than made up seed data. I'm assuming you can do things like dev migrations to seed the database with real data, but I'm not too familiar with the modern solutions to how you try to sync a production database back to a dev copy without syncing too much, like privacy or security data. |
Also, we have the staging domain, so we could merge this PR in, or else checkout the PR branch on stage, and give it a soft-run on that domain by pointing the consuming apps at the stage API enpoints. |
So for Organizations, we need to hash out some of these details, and thank you for pointing me to the docs, I did not see that. If everyone thinks this is a good idea, and we can talk about this on the hack night, I will go ahead and add those filters. But the reality is that, we will merge this PR, offer event-api first, and then finish up the orginization api + admin to manage it. |
I have confirmed that this implementation of the events API is compatible with the current state of the Slack bot. I haven't confirmed that this is the case with the following: From a cursory comparison of key-values I think things will go prettty smoothly. I love the idea of placing this into a staging environment and gradually testing there- I love that there is that option. |
We've never really used the create, update or delete features of the CRUD parts of the orgs API, so I wouldn't worry about supporting anything beside Read, unless we have some future idea where authenticated CUD is needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bogdankharchenko thanks, I see that the max_days_in_past and the new max_days_in_the_future are implemented.
The other config value was default_days_in_the_past, which we had set to 1 day in the past so when a consuming app asked for upcoming events, without specifying a start date, it would still show some past events. This was mainly useful so a dashboard like at OpenWorks didn't roll off an event after it started. It might also be useful in situations like a multi-day conference when using the start_date of "now" in response to an API request might cause something that's ongoing to disappear before it actually finished.
The "default" days, was if no start and end date were specified by an API request. It would usually return events from yesterday and into the future.
Thoughts on that config value?
'status' => 'all', | ||
'order_by' => 'start_desc', | ||
'expand' => 'event_sales_status', | ||
'start_date.range_start' => now()->subMonths(1)->format('Y-m-d\TH:i:s'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thought is if we'd run the risk of hitting API throttle limits if we are asking Meetup or Eventbrite for way more data than we need. For instance, Eventbrite has headers with a requests per hour, but I'd be surprised if either of these services didn't have some undocumented volume limits to avoid apps doing things like inadvertently requesting 10 years of data every 15 minutes.
The defaults are 30, and 180 days. See |
I just took the latest changes for a test drive and they work great! Nicely done!! I've left a single comment about .env.sample, but that was it. |
Co-authored-by: Matthew Thornton <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Emphatically re-approving!
…e-cancellation-attr-tweak Tweak Eventrbrite Response Attribute Name for Cancellation Detection
@bogdankharchenko are there any commands we need to run one-time with this PR? And, are there any new commands we'd want to add to scripts/handle-deploy-update.sh deployment script for regular execution? |
php artisan pull:orgs
php artisan import:events These commands will "prime the pump", and then events will be imported every 3 hours from that point forward. Based on my experience locally the only other command I had to execute was |
OK so one thing that is not mentioned in this PR but its critical. We are adding a relationship between the events table and the organizations. Currently we do not have a way to map old events to organizations (in laravel) -- as soon as we deploy this pr, a few things will break.... To mitigate this, we need to wipe our events and organizations from the database, and then run the commands to import organizations and events. We can either do,
Obvious downsides are we lose old events from the database, but the event-importer will importer will attempt to go back 30 days and import those events again. |
@bogdankharchenko there's no worry about losing past events. We've done it many times to try to fix other issues. Would one of the existing database commands be sufficient, like db:wipe? |
I've set up a small command to wipe just those two tables: bogdankharchenko#3
|
Kudos to @bogdankharchenko for the work and Matt for the heavy lifting on the review and testing. This is now staged. https://stage.hackgreenville.com/events The only minor hiccup I've seen so far is remembering to copy the .env.example variables to .env, setting the Eventbrite private, key and then clearing the config cache file because it wasn't recognizing the new .env vars.
The events data does differ slightly from stage and live, but we've always had a bunch of unknown cancelled events, so it's possible / likely the new API is properly avoiding an issue with the existing events API. It's worth comparing stage to live before we point some of the consuming apps to the stage copy for the next level of testing. |
OK This is a big one... 🔥 🔥 🔥
Why
We have been talking about migrating + improving the events api. I figured I would take a stab at this. I think having this code inside Laravel would give us an ability to move forward very quickly with a v2 of the api, along with other wishlist items.
Events + Organizations API Support
These new API Endpoints have the same exact data structure as the current events.openupstate.org and organizations.openupstate.org - meaning we could simply change the URL's on any of the applications which depend on the openupstate api's to the hackgreenville.com api's and everything should continue to work as intended (some small tweaks may be necessary)
Simply put, we use the
php artisan pull:orgs
command to pull all the organization data and store it on hackgreenville’s database.We no longer need to “pull:events” see “Event Importer” below
Once we deploy this - we should do a 301 redirect from the two api endpoints to point to hackgreenville’s api.
API Endpoints
https://hackgreenville.com/api/v0/events
https://hackgreenville.com/api/v0/orgs
Side note
Event Importer
Event Importer replicates the same results as the events-api python project - but it stores it into hackgreenville’s event table - we no longer need to import events from openupstate api via
php artisan pull:events
One of the biggest benefits, is that we can easily scale this to handle other event service providers, such as GetTogether or any other system. It is by far a much more robust system, which supports pagination and is backed by a test suite.
Currently supports:
Modular System
One of the things which is notable in this PR, is an introduction of a Modular code structure - see https://github.com/InterNACHI/modular
I work for InterNACHI and we have developed this package to be able to properly manage large code bases. This is a great way to “organize code” which will come into play as this repo grows and new comers look at our code.
Near Future
Local Testing
EVENTBRITE_PRIVATE_TOKEN
to .envEVENT_IMPORTER_MAX_DAYS_IN_PAST
to .envEVENT_IMPORTER_MAX_DAYS_IN_FUTURE
to .envphp artisan migrate:fresh
php artisan pull:orgs
php artisan import:events
TODO
Let’s Chat
I’m all ears. Hit me up on slack or dump a comment below. There are a few, “fix me” and “todo” comments, worth discussing.