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

GearmanClient::do*('namespace.name', '') error #113

Open
orolyn opened this issue Jun 26, 2014 · 8 comments
Open

GearmanClient::do*('namespace.name', '') error #113

orolyn opened this issue Jun 26, 2014 · 8 comments

Comments

@orolyn
Copy link
Contributor

orolyn commented Jun 26, 2014

When sending a job without a workload, or a workload with the default value of empty string, this error occurs:

Warning: GearmanClient::doNormal(): add_task(GEARMAN_INVALID_ARGUMENT) invalid workload -> libgearman/add.cc:199

This does no happen when a workload of integer 0 is sent.
This is an issue for jobs that do not require arguments.

Since the default arguments in the wrapping client is an empty string, I am assuming this is an issue related to my version of php5-gearman. Either way, is it feasible to alter the default params and prevent empty strings from being entered:

$params = empty($params) ? 0 : $params;
@orolyn
Copy link
Contributor Author

orolyn commented Jul 10, 2014

Not major, it may be an issue with the daemon.

@orolyn orolyn closed this as completed Jul 10, 2014
@bmeynell
Copy link
Contributor

👍 but I'd like to see the gearman docs about this. BTW why was this closed?

@orolyn
Copy link
Contributor Author

orolyn commented Jul 10, 2014

https://github.com/jeffreyhorner/gearman/blob/master/src/add.cc#L134

I have a feeling gearman doesn't like blank workloads, so I don't really know what can be done about it on this end.

@bmeynell
Copy link
Contributor

I personally don't like getting errors directly from the daemon like Warning: GearmanClient::doNormal(): add_task(GEARMAN_INVALID_ARGUMENT) invalid workload -> libgearman/add.cc:199 ... @mmoreram should this bundle catch this situation instead and throw its own exception?

@bmeynell
Copy link
Contributor

... or add a setting that automatically puts in a 0 for blank workloads ...

@orolyn orolyn reopened this Jul 10, 2014
@mmoreram
Copy link
Owner

Add the default is, imho, the best option, provided it is well documented. Creating a new Exception to override original Gearman maybe is not the best option. What do you think @bmeynell @orolin ?

@orolyn
Copy link
Contributor Author

orolyn commented Jul 10, 2014

Its tricky. I was just testing it with default values of integer 0. It worked for enqueue, but it looks like after running tasks with parameters of either 0 or empty string, we run into the same problem.

@orolyn
Copy link
Contributor Author

orolyn commented Jul 10, 2014

It might be worth making it a required argument.
Judging from gearman-server's source, the argument is expected to be a string:

gearman_argument_t gearman_argument_make(const char *name, const size_t name_length, const char *value, const size_t value_size)
{
  (void)name;
  (void)name_length;
  gearman_argument_t arg= { {0, 0}, { value, value_size }};

  return arg;
}

And it goes on to test the string length of the argument, at which point it throws the error.
I don't know how the do* methods worked with just a 0, but the addTask* doesn't.

// add_task
if ((gearman_size(workload) && gearman_c_str(workload) == NULL) or (gearman_size(workload) == 0 && gearman_c_str(workload)))
{
    gearman_error(client.universal, GEARMAN_INVALID_ARGUMENT, "invalid workload");
    return NULL;
}

Or:

// add_reducer_task
if ((gearman_size(workload) and not gearman_c_str(workload)) or (gearman_size(workload) == 0 && gearman_c_str(workload)))
{
    gearman_error(client->universal, GEARMAN_INVALID_ARGUMENT, "invalid workload");
    return NULL;
}

They look like they are doing the same thing, but are a little inconsistent.

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

No branches or pull requests

3 participants