新增auth支持 #328
新增auth支持 #328
Conversation
|
One of the changes here seems helpful, namely the use of PID tracking to ensure better "thread safety" across forks. That said, all the rest are breaking changes. Some bugfixes are reverted, and many features are removed. I suspect the changes in this PR were made to a much older version of the code than the most recent version in Additionally, your second commit changes the namespace of the package, which cannot be accepted. To address this, please close this PR, create a new branch even with the |
| @@ -1,4 +1,7 @@ | |||
| <?php | |||
| require_once dirname(__FILE__) . '/Resque/Event.php'; | |||
| require_once dirname(__FILE__) . '/Resque/Exception.php'; | |||
|
|
|||
There was a problem hiding this comment.
These lines are from before Composer was introduced...
| * and implement "thread" safety to avoid race conditions. | ||
| */ | ||
| protected static $pid = null; | ||
|
|
There was a problem hiding this comment.
This seems useful, though.
| * @param mixed $server Host/port combination separated by a colon, DSN-formatted URI, or | ||
| * a callable that receives the configured database ID | ||
| * and returns a Resque_Redis instance, or | ||
| * @param mixed $server Host/port combination separated by a colon, or | ||
| * a nested array of servers with host/port pairs. |
There was a problem hiding this comment.
Dropping a lot of valid server definitions, here...
| } | ||
|
|
||
| return $pid; | ||
| self::$redis->select(self::$redisDatabase); | ||
| return self::$redis; | ||
| } |
There was a problem hiding this comment.
The Resque::fork() method is pretty essential to the way the library operates now. It shouldn't be removed.
| return false; | ||
| } | ||
| return true; | ||
| self::redis()->rpush('queue:' . $queue, json_encode($item)); | ||
| } |
There was a problem hiding this comment.
This removes a lot of error handling...
| $counter = self::size($queue); | ||
| $result = self::redis()->del('queue:' . $queue); | ||
| return ($result == 1) ? $counter : 0; | ||
| } |
There was a problem hiding this comment.
Again, we still need to be able to remove jobs from the queue.
| public static function generateJobId() | ||
| { | ||
| return md5(uniqid('', true)); | ||
| } |
There was a problem hiding this comment.
Not strictly vital, but consistency is nice, so putting this in a method rather than duplicating it everywhere is helpful.
| } | ||
|
|
||
| return $pid; | ||
| self::$redis->select(self::$redisDatabase); | ||
| return self::$redis; |
There was a problem hiding this comment.
All of this logic is unnecessary with Credis, and is in fact redundant. Assuming it dates from before the switch to Composer, as well.
| $pid = getmypid(); | ||
| if (self::$pid !== $pid) { | ||
| self::$redis = null; | ||
| self::$pid = $pid; |
There was a problem hiding this comment.
This bit still looks helpful, though.
| { | ||
| self::$redisServer = $server; | ||
| self::$redisDatabase = $database; | ||
| self::$auth = $auth; |
There was a problem hiding this comment.
I can see why this might be tempting to have. Feels a bit superfluous, though.
|
I'm sure this is obvious in the code, but the PR title translates to Added auth support |
No description provided.