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

Cardkingdom import #5

Open
wants to merge 59 commits into
base: master
Choose a base branch
from
Open

Cardkingdom import #5

wants to merge 59 commits into from

Conversation

srhbutts
Copy link
Contributor

@srhbutts srhbutts commented Jun 1, 2019

Submitted initial version for review / suggestions?

@krainboltgreene
Copy link
Member

This should probably be two services, one for buy list and one retail?

srhbutts and others added 23 commits June 1, 2019 04:15
…ctions into pr-5/difference-engineers/cardkingdom_import
@srhbutts
Copy link
Contributor Author

srhbutts commented Jul 28, 2019

Requesting a review / more comments

Things I still have to add:

  • Some sort of output/logging to make sure it's actually processing
  • Still need to handle CardKingdom's goofy formatting for special cards (buy a box promo, judge promo, etc)
  • Split into retail/buylist services (if that's still suggested-- they'll both be using a lot of the same code)
  • Find a way to update certain categories more frequently than others; for example, 4th Edition buylist/retail prices are less likely to change than those from the latest set.
  • Make sure we time out requests to not hammer their servers / get banned...
  • Try to make our requests look more human?
  • Actually insert stuff into the DB

cardkingdom-import/function.rb Show resolved Hide resolved
@srhbutts srhbutts dismissed krainboltgreene’s stale review August 1, 2019 02:26

(already added this after you suggested the change)

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.

None yet

2 participants