simonw / datasette

An open source multi-tool for exploring and publishing data
https://datasette.io
Apache License 2.0
9.46k stars 677 forks source link

publish heroku does not work on Windows 10 #858

Open simonlau opened 4 years ago

simonlau commented 4 years ago

When executing "datasette publish heroku schools.db" on Windows 10, I get the following error

  File "c:\users\dell\.virtualenvs\sec-schools-jn-cwk8z\lib\site-packages\datasette\publish\heroku.py", line 54, in heroku
    line.split()[0] for line in check_output(["heroku", "plugins"]).splitlines()
  File "c:\python38\lib\subprocess.py", line 411, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File "c:\python38\lib\subprocess.py", line 489, in run
    with Popen(*popenargs, **kwargs) as process:
  File "c:\python38\lib\subprocess.py", line 854, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "c:\python38\lib\subprocess.py", line 1307, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
FileNotFoundError: [WinError 2] The system cannot find the file specified

Changing https://github.com/simonw/datasette/blob/55a6ffb93c57680e71a070416baae1129a0243b8/datasette/publish/heroku.py#L54

to

line.split()[0] for line in check_output(["heroku", "plugins"], shell=True).splitlines()

as well as the other check_output() and call() within the same file leads me to another recursive error about temp files

simonw commented 4 years ago

I really need to get myself a Windows 10 development environment working so I can dig into this kind of bug properly. I have a gaming PC lying around that I could re-task for that.

smithdc1 commented 4 years ago

Hi Simon,

Thanks so much for all your work on datasette, it's an excellent project and I wish you all the best with it. I particularly enjoyed your talk at the Django London Meetup a short while back.

I've been trying to publish to Heroku from Windows 10 and I was running into this error. I'm not sure why it can't be run without shell=True on Windows but this seems to help. With this change, I am able to publish if I pass in a name to the publish command. When a name is not passed the default of datasette is used and therefore this line here fails (as datasette at heroku already exists) and causes the recession error mentioned above.

https://github.com/simonw/datasette/blob/9a6d0dce282e7fb58c5610e24c74098c923abfdc/datasette/publish/heroku.py#L126

I tried to write a patch for this but I am really struggling with being on Windows (many of the tests seem to fail anyway?), and my lack of knowledge of Mock, so sorry for this. Hope this is of some help.

robroc commented 3 years ago

@smithdc1 Can you tell us what you did to get it to publish in Windows? What commands did you pass?

smithdc1 commented 3 years ago

To get it to work I had to:

robroc commented 3 years ago

Apologies if I sound dense but I don't see where you would pass 'shell=True'. I'm using the CLI installed via pip.

On Sun., Mar. 7, 2021, 2:15 a.m. David Smith, notifications@github.com wrote:

To get it to work I had to:

-

add shell=true to the various commands in datasette

use the name argument of the publish command. ( https://docs.datasette.io/en/stable/publish.html)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/simonw/datasette/issues/858#issuecomment-792230560, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJJTOMZMGYSCGUU4J3AVSDTCMRX5ANCNFSM4ODNEDYA .

rachelmarconi commented 3 years ago

shell=True is added to line 56 (I guess it used to be 54) of heroku.py as detailed in the original issue. (for posterity)

rachelmarconi commented 3 years ago

any fixes for that recursive issue with temp file? I get it using both heroku and cloudrun, although it seems to still publish and deploy fine