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

improve usability with Docker #71

Merged
merged 4 commits into from
Apr 25, 2024
Merged

improve usability with Docker #71

merged 4 commits into from
Apr 25, 2024

Conversation

danthe1st
Copy link
Contributor

@danthe1st danthe1st commented Mar 18, 2024

Currently, writing/changing articles with the Docker tooling is a bit annoying as one always needs to rebuild the image when changing something which also requires the npm install to execute again.

This PR performs the following changes:

  • It reorders the operations in the Dockerfile to run npm install before copying the files resulting in further executions being able to use a cached image and not run npm install again when changing a file.
  • For consistency, inline codeblocks have been added to the Docker instructions in the README.md.
  • --rm is added to the instructions in the README.md in order to automatically remove the container after it terminates.
  • Instructions for starting it with volumes have been added to the README.md. If this command is used and an article is added after starting the server, it automatically reloads the site.

potential issues

  • Windows: On Windows, $PWD may not be available. It might be a good idea to add instructions for that.

other considerations

Since I added --rm, it would be possible to give the container a name using the --name option.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Mar 18, 2024
@carimura carimura self-assigned this Mar 19, 2024
@carimura
Copy link
Member

thanks! I've been out on paternity leave but will review and get this merged soon!

@danthe1st
Copy link
Contributor Author

It may also be useful to add -it --init in order to stop the container with Ctrl+C.

@carimura
Copy link
Member

ok overall these seem like good changes, but I'm not sure if the volume mount is going to work with gulp. Have you tested it? Does the site rebuild in the Docker container every time the file is modified on your file system?

@danthe1st
Copy link
Contributor Author

danthe1st commented Apr 24, 2024

I tested this on my Ubuntu system (I actually use this setup for writing articles except that I also use --init -it as mentioned in the description).
In my system, it does automatically rebuild when saving a file.

@carimura
Copy link
Member

carimura commented Apr 24, 2024

hmm.. I can't get it to work on a Mac. So, you see the rebuild in the Docker output when you change a file locally? Must be a browsersync issue.... might be related to this browsersync output:

❯ docker run --publish 3000:3000 --init -it --rm -v $PWD/app:/app/app devjava
[13:03:39] Using gulpfile /app/gulpfile.js
[13:03:39] Starting 'serve'...
[13:03:39] Finished 'serve' after 13 ms
[Browsersync] Access URLs:
 -----------------------------------
       Local: http://localhost:3000
    External: http://172.17.0.2:3000
 -----------------------------------
          UI: http://localhost:3001
 UI External: http://localhost:3001
 -----------------------------------
[Browsersync] Serving files from: ./site/
[Browsersync] Couldn't open browser (if you are using BrowserSync in a headless environment, you might want to set the open option to false)

@carimura
Copy link
Member

I've confirmed that the volume mount works fine, when I change a file in my local app directory it’s "changed" in the container, but the build doesn't trigger.

@danthe1st
Copy link
Contributor Author

danthe1st commented Apr 24, 2024

For me, it automatically rebuilds. Did you run the docker build command before?
This happens when changing a file after executing docker run (I opened it in the browser using http://localhost:3000 before changing the file):

dan1st@dan1st-laptop:/tmp/devjava-content$ docker run --publish 3000:3000 --init -it --rm -v $PWD/app:/app/app devjava
[13:31:25] Using gulpfile /app/gulpfile.js
[13:31:25] Starting 'serve'...
[13:31:25] Finished 'serve' after 19 ms
[Browsersync] Access URLs:
 -----------------------------------
       Local: http://localhost:3000
    External: http://172.17.0.2:3000
 -----------------------------------
          UI: http://localhost:3001
 UI External: http://localhost:3001
 -----------------------------------
[Browsersync] Serving files from: ./site/
[Browsersync] Couldn't open browser (if you are using BrowserSync in a headless environment, you might want to set the open option to false)
[13:32:04] Starting 'cleanup'...
[13:32:04] Finished 'cleanup' after 104 ms
[13:32:04] Starting 'copy_assets'...
[13:32:04] Finished 'copy_assets' after 98 ms
[13:32:04] Starting 'sassify'...
[13:32:07] Finished 'sassify' after 2.43 s
[13:32:07] Starting 'pre_process_pages'...
Adding tutorial new_features.virtual_threads to single author: CayHorstmann
Adding tutorial java-cert-overview to single author: JeanneBoyarsky
Adding tutorial debugging to single author: JeanneBoyarsky
Adding tutorial refactoring to single author: VenkatSubramaniam
Adding tutorial refactoring.simple.loops to single author: VenkatSubramaniam
Adding tutorial refactoring.loops.withsteps to single author: VenkatSubramaniam
Adding tutorial refactoring.foreach.withif to single author: VenkatSubramaniam
Adding tutorial refactoring.iteration.withtransformation to single author: VenkatSubramaniam
Adding tutorial lang.classes.enums to single author: DanielSchmid
Adding tutorial api.reflection to single author: DrHeinzM.Kabutz
Adding tutorial javafx.fundamentals to multiple authors: GailC.Anderson,PaulAnderson
Adding tutorial javafx.fundamentals.structure to multiple authors: GailC.Anderson,PaulAnderson
Adding tutorial javafx.fundamentals.layout to multiple authors: GailC.Anderson,PaulAnderson
Adding tutorial javafx.fundamentals.effects to multiple authors: GailC.Anderson,PaulAnderson
Adding tutorial javafx.fundamentals.properties to multiple authors: GailC.Anderson,PaulAnderson
Adding tutorial javafx.fundamentals.fxml to multiple authors: GailC.Anderson,PaulAnderson
Adding tutorial javafx.fundamentals.all to multiple authors: GailC.Anderson,PaulAnderson
[13:32:07] Finished 'pre_process_pages' after 66 ms
[13:32:07] Starting 'pages'...
Page new_features.using_preview resolved to undefined
Page lang.object resolved to undefined
Page lang.numbers_strings.numbers resolved to undefined
Page lang.numbers_strings resolved to undefined
Page lang.numbers_strings resolved to undefined
Page lang.basics.flow resolved to undefined
Page lang.classes.classes resolved to undefined
Page lang.classes-objects.switch-expression resolved to undefined
Page lang.classes-objects.switch-expression resolved to undefined
Page lang.records resolved to undefined
Javadoc RotationTransition resolved to undefined
[Browsersync] 51 files changed (index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html)
[13:32:07] Finished 'pages' after 380 ms
[Browsersync] Reloading Browsers... (buffered 51 events)
[13:32:10] Starting 'cleanup'...
[13:32:10] Finished 'cleanup' after 17 ms
[13:32:10] Starting 'copy_assets'...
[13:32:10] Finished 'copy_assets' after 63 ms
[13:32:10] Starting 'sassify'...
[13:32:12] Finished 'sassify' after 2.06 s
[13:32:12] Starting 'pre_process_pages'...
Adding tutorial new_features.virtual_threads to single author: CayHorstmann
Adding tutorial java-cert-overview to single author: JeanneBoyarsky
Adding tutorial debugging to single author: JeanneBoyarsky
Adding tutorial refactoring to single author: VenkatSubramaniam
Adding tutorial refactoring.simple.loops to single author: VenkatSubramaniam
Adding tutorial refactoring.loops.withsteps to single author: VenkatSubramaniam
Adding tutorial refactoring.foreach.withif to single author: VenkatSubramaniam
Adding tutorial refactoring.iteration.withtransformation to single author: VenkatSubramaniam
Adding tutorial lang.classes.enums to single author: DanielSchmid
Adding tutorial api.reflection to single author: DrHeinzM.Kabutz
Adding tutorial javafx.fundamentals to multiple authors: GailC.Anderson,PaulAnderson
Adding tutorial javafx.fundamentals.structure to multiple authors: GailC.Anderson,PaulAnderson
Adding tutorial javafx.fundamentals.layout to multiple authors: GailC.Anderson,PaulAnderson
Adding tutorial javafx.fundamentals.effects to multiple authors: GailC.Anderson,PaulAnderson
Adding tutorial javafx.fundamentals.properties to multiple authors: GailC.Anderson,PaulAnderson
Adding tutorial javafx.fundamentals.fxml to multiple authors: GailC.Anderson,PaulAnderson
Adding tutorial javafx.fundamentals.all to multiple authors: GailC.Anderson,PaulAnderson
[13:32:12] Finished 'pre_process_pages' after 27 ms
[13:32:12] Starting 'pages'...
Page new_features.using_preview resolved to undefined
Page lang.object resolved to undefined
Page lang.numbers_strings.numbers resolved to undefined
Page lang.numbers_strings resolved to undefined
Page lang.numbers_strings resolved to undefined
Page lang.basics.flow resolved to undefined
Page lang.classes.classes resolved to undefined
Page lang.classes-objects.switch-expression resolved to undefined
Page lang.classes-objects.switch-expression resolved to undefined
Page lang.records resolved to undefined
Javadoc RotationTransition resolved to undefined
[Browsersync] 51 files changed (index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html, index.html)
[13:32:13] Finished 'pages' after 227 ms
[Browsersync] Reloading Browsers... (buffered 51 events)

then, the site is automatically reloaded.

@carimura
Copy link
Member

@carimura
Copy link
Member

OK we solved it. had to enable usePolling in the watch command like so:

watch("app/**/*.md", {interval: 1000, usePolling: true}, series(build));

I think this change should be fine since any potential unknown effect is pretty limited.

@carimura
Copy link
Member

@danthe1st can you confirm that this still works on Ubuntu? If so we'll get this merged in ASAP.

Thank you!

@danthe1st
Copy link
Contributor Author

danthe1st commented Apr 25, 2024

Yes, I confirm it still works for me.
However, note that $PWD probably doesn't work with Windows' cmd and the -v $PWD/app:/app/app needs to be put before the image name (devjava) so that it is interpreted as an argument to docker and not an argument to gulp serve.

@carimura
Copy link
Member

OK @danthe1st reworded a bit. Going to get this merged now! Thanks again. If you want any change or addition from here just throw in another PR!

@carimura carimura merged commit 862c781 into java:main Apr 25, 2024
1 check passed
@danthe1st danthe1st deleted the docker branch April 25, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants