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 explicit whitelist of RPC commands for luigid #1631

Merged
merged 1 commit into from Apr 5, 2016
Merged

Add explicit whitelist of RPC commands for luigid #1631

merged 1 commit into from Apr 5, 2016

Conversation

ghost
Copy link

@ghost ghost commented Apr 5, 2016

Limit calls on Scheduler from API to only those required for functionality.

Basic functionality testing was conducted, but not extensive testing.

@erikbern
Copy link
Contributor

erikbern commented Apr 5, 2016

nice!

'graph',
'inverse_dep_graph',
'ping',
'prune',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this actually should be public. But whatever.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify. I'm not sure it makes sense for clients to be able to call prune. Many other of the API calls does call prune internally themselves, like graph, inverse_dep_graph etc.

This was just a minor comment so I just merged right away.

@Tarrasch Tarrasch merged commit 3ad5bbb into spotify:master Apr 5, 2016
@Tarrasch
Copy link
Contributor

Tarrasch commented Apr 5, 2016

Nice! Thanks :)

Tarrasch pushed a commit that referenced this pull request Apr 7, 2016
This fixes a bug resulting from the interference of PR #1625 (Task status messages) and PR #1631 (Add explicit whitelist of RPC commands for luigid) task status messages.

To fix this, I simply added ``set_task_status_message`` and ``get_task_status_message`` to the white-listing.
Tarrasch added a commit that referenced this pull request Sep 15, 2016
As of #1631 this is fixed, later #1734 enhanced it
Tarrasch added a commit that referenced this pull request Sep 16, 2016
As of #1631 this is fixed, later #1734 enhanced it
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.

2 participants