Skip to content
This repository has been archived by the owner on Aug 1, 2022. It is now read-only.

Additions in forks #2

Open
paulirish opened this issue May 5, 2016 · 12 comments
Open

Additions in forks #2

paulirish opened this issue May 5, 2016 · 12 comments

Comments

@paulirish
Copy link

I'm about to set this up.. and just noticed some nice work in various forks.. just FYI for you

thanks for the project!

@davidofwatkins
Copy link
Owner

Awesome! Wow, thanks for bringing these to my attention, @paulirish! I'll get these great changes merged in when I can :)

And as always, feel free to open a PR if you're making changes of your own!

@paulirish
Copy link
Author

And then over in my fork I made a few changes (some that break the cron & email flow)

  • can check multiple locations
  • better error reporting
  • mapping location IDs to readable names

image

downside is the python doesn't yet handle the multi-location results from the phantom script

but other than that it's feeling p good

@nnja
Copy link
Contributor

nnja commented May 30, 2016

I've made extensive changes in my fork as well.

I haven't documented them yet, but in summary:

  • Fully refactored the python code to make it easier to understand and more extensible
  • Added support for flags, added the following flags:
    1. config for sending through a gmail account, so you don't have to set up sendmail locally
    2. no mail flag
    3. osx specific: notify via the notification center when a new appointment is found
  • Since cron isn't well supported on osx, I added a plist file for an osx launch daemon which is currently configured to run every 30 minutes. Instructions for setting up the daemon are currently in a comment at the top of the file.

@paulirish When I have time next week, I can pull in your latest changes on the phantomjs side and make updates to the python script to support multiple locations.

@davidofwatkins The pull request #3 I opened 2 weeks ago with small bug fixes hasn't been acknowledged, so if you're interested in merging any of these changes feel free to reach out to me.

@davidofwatkins
Copy link
Owner

@nnja Thanks for submitting the PR! Apologies for not responding yet; I've been swamped with other work recently, but I'll try to take a look as soon as I can! :)

@nnja
Copy link
Contributor

nnja commented Jun 1, 2016

@davidofwatkins - Would you like me to open a PR with the bigger changeset including the features I mention above? The additions I made are backwards compatible with your branch.

@paulirish
Copy link
Author

@nnja you're a champion! thanks for sorting this out. :)

@davidofwatkins
Copy link
Owner

@nnja Awesome, yes that would be great. All sound like good improvements.

@pixitha
Copy link

pixitha commented Sep 27, 2016

@nnja did you ever get that plist stuff done? I don't see anything about that in the files...

@nnja
Copy link
Contributor

nnja commented Sep 27, 2016

@pixitha Yes, along with some refactoring so I had an easier time working on the code - but there were conflicts before I had an opportunity to open a PR and I don't have time to work on this anymore.

If you want to take a look, my branch is here: https://github.com/nnja/ge-cancellation-checker

I added some additional flags:

parser.add_argument('--no-email', action='store_true', dest='no_email', default=False, help='Don\'t send an e-mail when the script runs.')
parser.add_argument('--use-gmail', action='store_true', dest='use_gmail', default=False, help='Use the gmail SMTP server instead of sendmail.')
parser.add_argument('--notify-osx', action='store_true', dest='notify_osx', default=False, help='If better date is found, notify on the osx desktop.')

The plist is here with instructions for how to run it as a comment at the top of the file.

Unfortunately I never did update the README, so it's a big ad-hoc.

Hope that helps!

@nnja
Copy link
Contributor

nnja commented Sep 27, 2016

@pixitha I'm mistaken and the changes with the flags were merged in, just not the plist.

I'll submit a proper PR for the plist when I have time, as well as updates to the README with instructions.

In the meantime, this should be enough to get you started.

@pixitha
Copy link

pixitha commented Sep 29, 2016

Dumb question about the plist, having issues getting it to work correctly.

When I load the plist, it runs the python script just fine, but then python appears to have issues locating phantomjs, even though I can run everything by hand just fine?

09/28/2016 09:32:05 PM Running GE cron with arguments: {'no_email': False, 'configfile': '/Users/xxxxx/ge-cancellation-checker/config.json', 'notify_osx': True, 'use_gmail': False}
09/28/2016 09:32:05 PM Something went wrong when trying to run ge-cancellation-checker.phantom.js. Is phantomjs is installed?

@nnja
Copy link
Contributor

nnja commented Oct 5, 2016

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

No branches or pull requests

4 participants