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

Error parsing args when use_papa = True #930

Closed
cgarciaarano opened this issue Sep 16, 2015 · 16 comments
Closed

Error parsing args when use_papa = True #930

cgarciaarano opened this issue Sep 16, 2015 · 16 comments

Comments

@cgarciaarano
Copy link
Contributor

Hi,

we've found that Circus 0.12.2 is not parsing args correctly when use_papa = True. With this config file:

[circus]
check_delay = 5
endpoint = tcp://127.0.0.1:5555
pubsub_endpoint = tcp://127.0.0.1:5556
statsd = true

[watcher:app1]
cmd = ping
args = 8.8.8.8
use_papa=True

The debug log shows:

2015-09-16 19:35:57 circus[17841] [DEBUG] cmd: ping
2015-09-16 19:35:57 circus[17841] [DEBUG] args: ['8', '.', '8', '.', '8', '.', '8']
2015-09-16 19:35:57 circus[17841] [DEBUG] process args: ['ping', '8', '.', '8', '.', '8', '.', '8']

and the process keeps flapping.

Commenting out in papa_process_proxy.py the self.args assign:

    def spawn(self):
        # noinspection PyUnusedLocal
        socket_names = set(socket_name.lower()
                           for socket_name in self.watcher._get_sockets_fds())
        self.cmd = self._fix_socket_name(self.cmd, socket_names)
        #self.args = [self._fix_socket_name(arg, socket_names)
        #             for arg in self.args]
        args = self.format_args()
        stdout = _bools_to_papa_out(self.pipe_stdout, self.close_child_stdout)
        stderr = _bools_to_papa_out(self.pipe_stderr, self.close_child_stderr)

seems to fix this behaviour, because args is being processed as a string, not a list. I'm not really sure what _fix_socket_name() is doing. Maybe this fix is enough?

self.args = [self._fix_socket_name(arg, socket_names)
                     for arg in self.args.split(' ')]

Regards

@ayashjorden
Copy link

Hi, @k4nar,
I'm having the same issue with version 0.12.1 .
Any time frame for the fix?

Thank you,
Yarden

@k4nar
Copy link
Contributor

k4nar commented Nov 30, 2015

@scottkmaxwell : Can I assign you to this one ?

@cgarciaarano
Copy link
Contributor Author

I can pull request my proposal, but I'm not really sure it's correct, it could have side effects.

@k4nar
Copy link
Contributor

k4nar commented Nov 30, 2015

We are already splitting the args at some point, so I think that self._fix_socket_name should be applied at this point.

@scottkmaxwell
Copy link
Contributor

Yeah, it sounds like I was expecting args to already be a list at this point. I am completely swamped at the moment but I can try to have a look toward the end of the week if somebody hasn't figured out a proper fix yet.

@ayashjorden
Copy link

@scottkmaxwell any update here?

@scottkmaxwell
Copy link
Contributor

Sorry this took so long.

@ayashjorden
Copy link

@k4nar when can we expect a release containing this fix?

@k4nar
Copy link
Contributor

k4nar commented Apr 25, 2016

@ayashjorden : Sorry for the delay, I'll try to make a new release this week.

@ayashjorden
Copy link

Thank you @k4nar :)

@antgel
Copy link

antgel commented Jun 8, 2016

Hey all, has the fix been released yet?

@k4nar
Copy link
Contributor

k4nar commented Jun 8, 2016

Sorry, I've been experiencing some troubles with our tests suite (cf #984 for progress), and I would really like to fix that before releasing a new version. I'll push as much effort as I can to make it happen this week.

@k4nar
Copy link
Contributor

k4nar commented Jul 5, 2016

I've merged #984. I'm giving some time to the opened PRs to be rebased & merged, and I'll release a new version.

@ayashjorden
Copy link

Thank you for the update, looking forward to it :)

On Tue, Jul 5, 2016 at 2:13 PM, Yannick PÉROUX notifications@github.com
wrote:

I've merged #984 #984. I'm
giving some time to the opened PRs to be rebased & merged, and I'll release
a new version.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#930 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AEAnU2IKjlNl0Rdss-09wEQlogZ5ltM8ks5qSjxQgaJpZM4F-l60
.

@k4nar
Copy link
Contributor

k4nar commented Aug 12, 2016

@ayashjorden : I just released Circus 0.14. Sorry for the (very) long delay!

@k4nar k4nar closed this as completed Aug 12, 2016
@ayashjorden
Copy link

Thank you! @k4nar

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

No branches or pull requests

5 participants