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

octoprint 1.3.0 is very picky about commandline order #1633

Closed
mrvanes opened this issue Dec 8, 2016 · 8 comments

Comments

Projects
None yet
4 participants
@mrvanes
Copy link

commented Dec 8, 2016

Thanks for that hard work and making 1.3.0 available!

Took me a while to find out that with the new 1.3.0 this commandline fails:
/usr/local/bin/octoprint --port 80 --iknowwhatimdoing daemon start

but this one works:
/usr/local/bin/octoprint daemon start --port 80 --iknowwhatimdoing

The logs aren't very vocal about the reason why it failed (it just stops, leaving a pid hanging around) but
/usr/local/bin/octoprint --port 80 --iknowwhatimdoing serve

hinted that Octoprint should not be run as root, so I suspected the --iknowwhatimdoing wasn't honoured in that order. Changing the order fixed "serve" and after that "daemon start" as well.

So, 1.3.0 is running nicely, but not without a fuss...

@GitIssueBot

This comment has been minimized.

Copy link
Collaborator

commented Dec 8, 2016

Hi @mrvanes,

It looks like there is some information missing from your bug report that will be needed in order to solve the problem. Read the Contribution Guidelines which will provide you with a template to fill out here so that your bug report is ready to be investigated (I promise I'll go away then too!).

If you did not intend to report a bug but wanted to request a feature or brain storm about some kind of development, please take special note of the title format to use as described in the Contribution Guidelines.

Please do not abuse the bug tracker as a support forum - if you have a question or otherwise need some kind of help or support refer to the Mailinglist or the G+ Community instead of here.

Also make sure you are at the right place - this is the bug tracker of the official version of OctoPrint, not the Raspberry Pi image OctoPi nor any unbundled third party OctoPrint plugins or unofficial versions. Make sure too that you have read through the Frequently Asked Questions and searched the existing tickets for your problem - try multiple search terms please.

I'm marking this one now as needing some more information. Please understand that if you do not provide that information within the next two weeks (until 2016-12-22 22:30 UTC) I'll close this ticket so it doesn't clutter the bug tracker. This is nothing personal, so please just be considerate and help the maintainers solve this problem quickly by following the guidelines linked above. Remember, the less time the devs have to spend running after information on tickets, the more time they have to actually solve problems and add awesome new features. Thank you!

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being, so don't expect any replies from me :) Your ticket is read by humans too, I'm just not one of them.

@foosel

This comment has been minimized.

Copy link
Owner

commented Dec 8, 2016

Ignore the bot, all fine with this issue (special case), will take a look tomorrow.

@foosel

This comment has been minimized.

Copy link
Owner

commented Dec 9, 2016

Ok, after taking a closer (not sleep deprived ;)) look, this actually looks like it's supposed to. --port etc are parameters that belong to the sub command daemon (and serve), so they belong AFTER that command. Look at it as a complete command (even though there's a space in between). For something like octoprint-daemon --port it would be completely clear why the parameters come after. octoprint daemon is basically the same idea, just without having to define multiple executables but just use one which supports subcommands.

I initially thought you'd run into problems with the old --daemon flag to trigger daemon mode instead of the daemon sub command, and there it would have been a bug, but that appears to work fine (or am I mistaken?).

The new CLI offers more possibilities for future extension, but the price for that is to also be a bit stricter about things to not drown in parameter chaos, so I'm a bit reluctant to make an exception for things like octoprint --port 80 daemon - it's somewhat backwards ("OctoPrint! Remember port 80, I'll tell you what to do with that info in a sec!") and changing it would set a bad precedent for maintainability in the long run (existing exceptions to the rule right now only are in place to stay backwards compatibility).

@nunofgs

This comment has been minimized.

Copy link

commented Dec 10, 2016

I'm on a raspberry pi and having this issue, but I can reproduce it even without arguments:

# octoprint
Traceback (most recent call last):
  File "/usr/local/bin/octoprint", line 11, in <module>
    load_entry_point('OctoPrint==1.3.0', 'console_scripts', 'octoprint')()
  File "/usr/local/site-packages/octoprint/__init__.py", line 398, in main
    from octoprint.cli import octo
  File "/usr/local/site-packages/octoprint/cli/__init__.py", line 124, in <module>
    from .dev import dev_commands
  File "/usr/local/site-packages/octoprint/cli/dev.py", line 232, in <module>
    @dev_commands.group(name="dev", cls=OctoPrintDevelCommands)
  File "/usr/local/site-packages/click/core.py", line 1169, in decorator
    cmd = group(*args, **kwargs)(f)
  File "/usr/local/site-packages/click/decorators.py", line 115, in decorator
    cmd = _make_command(f, name, attrs, cls)
  File "/usr/local/site-packages/click/decorators.py", line 89, in _make_command
    callback=f, params=params, **attrs)
  File "/usr/local/site-packages/octoprint/cli/dev.py", line 24, in __init__
    from octoprint.util.commandline import CommandlineCaller
  File "/usr/local/site-packages/octoprint/util/__init__.py", line 8
SyntaxError: __future__ statements must appear at beginning of file

Any ideas?

@foosel

This comment has been minimized.

Copy link
Owner

commented Dec 10, 2016

@nunofgs that doesn't look like the same problem. Please open a new ticket. Follow the instructions in the new ticket form please and provide information as requested. Include how you have OctoPrint installed in your system (this looks like a non standard install so please provide information on how you set this up). Also provide a copy of /usr/local/site-packages/octoprint/util/__init__.py in that ticket please because the line it appears to be complaining about there is not line 8 in actual OctoPrint 1.3.0 so on first glance something appears fishy with that install.

@mrvanes

This comment has been minimized.

Copy link
Author

commented Dec 11, 2016

Ah yes, the reason I bumped into this issue was because my original startup line in rc.local read:
/usr/local/bin/octoprint --port 80 --iknowwhatimdoing --daemon start

After seeing the deprecation warning of --deamon, I just replaced it with:
/usr/local/bin/octoprint --port 80 --iknowwhatimdoing daemon start

Because you know, admins are lazy. And that results in the mentioned problems. I agree that the command -> options paradigm is logical, it just causes issues for lazy people like me.

@foosel

This comment has been minimized.

Copy link
Owner

commented Dec 12, 2016

@mrvanes all right, going to close this then. Also, I guess you only noticed the deprecation warning in the first place since I managed to break octoprint --daemon accidentally (see #1641) :/ and stuff wasn't starting up properly anymore? That has now been fixed too.

@foosel foosel closed this Dec 12, 2016

foosel added a commit that referenced this issue Dec 16, 2016

Don't care about common params on CLI
--basedir, --config, --verbose, --safe may now come before or after
subcommands and should still be evaluated.

For the server commands (legacy, "server" and "daemon"), the same
should now hold true for the related parameters --host, --port, --debug,
--logging, --iknowwhatimdoing and also --pid (for daemon command).

While having the parameters belong to the individual commands and only
there (which is click's basic approach) is way more cleaner, too many people
were running into issues with that strict approach after all.

I just hope the somewhat hackish approach with context injection needed to
get the less strict version to work won't backfire badly in the long run.

See also #1633 and #1657
@foosel

This comment has been minimized.

Copy link
Owner

commented Dec 16, 2016

I revisited this after yet another person ran into issues with that.

octoprint --port 80 --iknowwhatimdoing daemon start and the like should now work. I sincerely hope that the decision to change that and going against the default behaviour of the underlying CLI parser won't bite me down the road.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.