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

Clear output folder setting, readme changes, Dockerfile changes #5

Merged
merged 12 commits into from
Aug 4, 2023
Merged

Clear output folder setting, readme changes, Dockerfile changes #5

merged 12 commits into from
Aug 4, 2023

Conversation

m-a-x-s-e-e-l-i-g
Copy link
Contributor

@m-a-x-s-e-e-l-i-g m-a-x-s-e-e-l-i-g commented Jul 25, 2023

  • Added setting to clear output folder.
  • Some readme changes that work for me
  • Undone some changes to Dockerfile to make it work
    I couldn't build with the previous version of the Dockerfile:
    On RUN apt-get install -y google-chrome-stable=${CHROME_VERSION} in Dockerfile:10
    I received: ERROR: failed to solve: process "/bin/sh -c apt-get install -y google-chrome-stable=${CHROME_VERSION}" did not complete successfully: exit code: 100
    image

@a-nau
Copy link
Owner

a-nau commented Jul 26, 2023

Thanks for the PR @m-a-x-s-e-e-l-i-g! At first glance:

  • Dockerfile works locally for me and also in Github CI. Are you using a very old version of Docker? Or did you maybe try to build an older version? I prefer it as is, since it fixes the Chrome version and prevents future incompatibilities.
  • To clean up the folder I would just remove it and create a new one, that's only 2 lines of code. Also pathlib is much more intuitiv than os, you should check it out. Also we don't need a separate file in the tools folder for this, I think.
  • haven't tried the button in the UI, but very good idea!
  • maybe split up build and run in the readme again?

@m-a-x-s-e-e-l-i-g
Copy link
Contributor Author

m-a-x-s-e-e-l-i-g commented Jul 28, 2023

Sorry, I didn't read the full error before:

=> ERROR [ 6/14] RUN apt-get install -y google-chrome-stable=114.0.5735.198-1                                                                                          1.5s 
------
 > [ 6/14] RUN apt-get install -y google-chrome-stable=114.0.5735.198-1:
0.471 Reading package lists...
1.242 Building dependency tree...
1.414 Reading state information...
1.420 Package google-chrome-stable is not available, but is referred to by another package.
1.420 This may mean that the package is missing, has been obsoleted, or
1.420 is only available from another source
1.420
1.422 E: Version '114.0.5735.198-1' for 'google-chrome-stable' was not found

It seems that older versions get removed from their PPA.

  • Fixed this issue in last commit.
  • I split up the build and run in readme
  • I'll try out pathlib, thanks!

@m-a-x-s-e-e-l-i-g
Copy link
Contributor Author

Removing the complete folder and recreating it using Pathlib and shutil could result in
OSError: [Errno 16] Device or resource busy: '/usr/src/app/output'
You can't keep your output or subdirectories open without having to catch the error.

I moved the function to frontend.py

@a-nau
Copy link
Owner

a-nau commented Aug 4, 2023

Cool, thanks! Looks good :) Sorry for the delay...

@a-nau a-nau merged commit a3a8af3 into a-nau:main Aug 4, 2023
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.

None yet

2 participants