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

Add trigger_async method #4

Closed
wants to merge 1 commit into from
Closed

Add trigger_async method #4

wants to merge 1 commit into from

Conversation

loris
Copy link
Contributor

@loris loris commented Sep 18, 2012

Add trigger_async method to allow asynchronous calls to the
Pusher REST API using fsockopen instead of cURL.
Also useful on shared hosts where cURL is not allowed/installed.

Add trigger_async method to allow asynchronous calls to the
Pusher REST API using fsockopen instead of cURL.
Also useful on shared hosts where cURL is not allowed/installed.
@leggetter
Copy link
Contributor

An async method would be really handy (and thanks for the earlier socket_id fix).

I'd really like to encourage tests to be submitted along with commits. It might be difficult to do since it touches some native functions.

There's also a reasonable amount of duplicate code here from the trigger method so it would be good to try and refactor some of that code out.

p.s. drop me an email phil@pusher.com for some Pusher credit as thanks.

@adamyeats-zz
Copy link
Contributor

@leggetter, is this good to merge?

@leggetter
Copy link
Contributor

Doesn't look like it can be automatically merged any more.

But, it is just one method so probably wouldn't be too difficult to copy & paste.

The reasons I didn't pull this in are (as above); no tests and duplicate code from the trigger function.

Not sure why I didn't just merge locally and do the refactoring myself. Hopefully I was busy and not just lazy.

Just spent a bit of time looking at this - which led me to set up stuff locally etc. etc. - and then when I got to this I noticed there also isn't a callback that states if the call fails/succeeds.

So, if we want an trigger_async I'd suggest:

  1. Tests
  2. Refactoring to remove duplicate code between trigger, trigger_async and create_curl
  3. trigger_async should provide a result callback of some sort - if possible(?)

@adamyeats-zz
Copy link
Contributor

@leggetter Sounds like something I can have a go at. Would you ming reviewing the PR once I push it?

@leggetter
Copy link
Contributor

@adamyeats No problem at all. Pleased to be part of the @pusher community.

@rakannimer
Copy link

I was thinking of pushing all pusher event trigger calls to a resque queue. Any thoughts on that ?

@leggetter
Copy link
Contributor

See #22

@leggetter leggetter closed this Dec 13, 2014
@NinoSkopac
Copy link

I don't understand how this can be asynchronous.

From http://php.net/manual/en/function.fsockopen.php:

The socket will by default be opened in blocking mode. You can switch it to non-blocking mode by using stream_set_blocking().

Searching for stream_set_blocking() inside this PRs source code yielded no results.

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

5 participants