[engine] Engine should automatically detect when classes and config are changed and reload the context #6223

russdanner · 2023-09-21T17:22:52Z

Latest version

The issue is in the develop branch
The issue is in the latest released 4.1.x
The issue is in the latest released 4.0.x
The issue is in the latest released 3.1.x

Duplicates

I have searched the existing issues

Is your enhancement request related to a problem? Please describe.

Today changes to nested groovy classes often require a context rebuild in order to get picked up. Ideally, CrafterCMS could detect a development change that would require a context rebuild and handle that seamlessly for the developer.

Example:
A rest controller imports a class written in groovy and uses it. The user changes the class but not the rest script. The updated class changes are not seen until after a context rebuild (or a change to the rest controller)

Describe the solution you'd like

Engine should automatically detect when classes and config are changed and reload the context.

russdanner · 2023-09-21T17:24:07Z

The goal is to allow the developer to modify all code and configuration and the test in Engine without needing to commit or manually execute any APIs. Groovy classes, Engine Config and Spring Config updates all require the developer to act in order for Engine to see their changes.`

phuongnq · 2023-09-25T11:38:09Z

@sumerjabri @russdanner The implementation enables the DirectoryWatcher for /scripts and /site/engine and for preview mode.
Should we have a separate flag to enable this instead of preview mode?

sumerjabri · 2023-09-25T16:07:52Z

@phuongnq the Deployer should not rebuild context for Preview targets now that we're doing that in Engine. See if that's possible, please.

sumerjabri · 2023-09-25T16:10:45Z

Suggested test cases:

Write/change/delete files to disk (no commit) in the monitored paths: /scripts/classes/.../*.java (a class file) -- see if Engine picks it up and it runs with the new code
Same for config files (Spring context updates, etc.)
Git pull similar files from remote where these files are updated

phuongnq · 2023-09-26T06:56:38Z

@sumerjabri Regarding the Deployer, since we use the local-target-template.yaml for the preview. I have removed the http processor to do the rebuild. (Assuming we use this template for preview only?).

Those test cases are tested in Linux. For the pull, since they will have the same modified date, I updated the logic to do just one rebuild instead of multiple rebuilds for each file since one rebuild would be enough. If the modified date are different for multiple files, we will treat it as multiple rebuild requests.

We may also want to test the same in MacOS since the library handle watcher differently in the MacOS.

sumerjabri · 2023-09-28T13:30:54Z

We're okay with the Deployer template.

The main issue is the algorithm. My suggested algorithm is:
Main idea: Trigger once for a large set of changes, but never wait more than 1 second
Ingredients:

A counter with a configurable limit (default 5)
A configurable timeout (default 200 milliseconds)

Alg:

When the watcher triggers, set a counter to 1 and sleep for 200 milliseconds.
Wake up and check:
- Is the counter 5 (one second has passed)? Then, trigger a rebuild.
- Has anything else changed (more changes since we slept)? If so, increment the counter and sleep for 200 milliseconds. If nothing has changed, then trigger a rebuild.
All of the above happens in the same thread, or a thread pool with limits is used.

Let me know your thoughts. I can also meet with you as needed.

sumerjabri · 2023-10-04T21:42:38Z

@yacdaniel you may start testing in develop

@phuongnq please port to 4.0

Euquimides · 2023-10-16T22:51:44Z

Hi @sumerjabri and @phuongnq ! While validating the ticket I noticed the following. Let's say we change the configuration of the new engine watcher to watch paths in general like:

# Engine file change watcher paths
crafter.engine.watcher.paths=\
  /scripts,\
  /config/engine,\
  /
# Engine file change watcher counter limit to rebuild
crafter.engine.watcher.counter.limit=5
# Engine file change watcher thread interval in milliseconds
crafter.engine.watcher.interval.period=200

The watcher will follow changes to /.git/ files, giving away something like this in the logs:

[INFO] 2023-10-16T16:27:16,477 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'MODIFY'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/site/website/index.xml'. Hash value: 'AAAAAGUtuMQAAAAAHFOFgw==' 
[INFO] 2023-10-16T16:27:16,479 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'CREATE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/index.lock'. Hash value: 'AAAAAGUtuMQAAAAAHFOFgw==' 
[INFO] 2023-10-16T16:27:16,480 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'CREATE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/objects/b6'. Hash value: 'DIRECTORY' 
[INFO] 2023-10-16T16:27:16,480 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'CREATE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/objects/b6/2c8d009a5ee7df6d74ef016292e38ac970eebb'. Hash value: 'AAAAAGUtuMQAAAAAHFOFgw==' 
[INFO] 2023-10-16T16:27:16,480 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'DELETE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/index.lock'. Hash value: 'none' 
[INFO] 2023-10-16T16:27:16,480 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'MODIFY'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/index'. Hash value: 'AAAAAGUtuMQAAAAAHFOFgw==' 
[INFO] 2023-10-16T16:27:16,482 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'CREATE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/index.lock'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,482 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'MODIFY'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/objects/b6/2c8d009a5ee7df6d74ef016292e38ac970eebb'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,483 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'CREATE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/objects/7c'. Hash value: 'DIRECTORY' 
[INFO] 2023-10-16T16:27:16,483 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'CREATE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/objects/7c/a1f6f5d0ee5c36a9f0286ff61b36e3db20fee2'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,483 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'CREATE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/objects/62/54aea307ece02a5583cbd6b4c2d81ea3562395'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,483 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'CREATE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/objects/6e/3e55a9994e40b99a11c1b6f58ee3dba0774839'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,483 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'CREATE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/next-index-66222.lock'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,483 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'MODIFY'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/COMMIT_EDITMSG'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,484 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'CREATE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/objects/20/tmp_obj_2YAa7G'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,484 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'CREATE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/HEAD.lock'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,484 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'MODIFY'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/refs/heads/master'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,484 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'CREATE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/objects/20/fe91798954972b7c3493b80ff52bee386952ae'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,484 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'DELETE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/objects/20/tmp_obj_2YAa7G'. Hash value: 'none' 
[INFO] 2023-10-16T16:27:16,484 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'MODIFY'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/logs/HEAD'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,484 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'MODIFY'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/logs/refs/heads/master'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,484 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'DELETE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/HEAD.lock'. Hash value: 'none' 
[INFO] 2023-10-16T16:27:16,484 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'DELETE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/index.lock'. Hash value: 'none' 
[INFO] 2023-10-16T16:27:16,484 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'MODIFY'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/index'. Hash value: 'AAAAAGUtuMQAAAAAHJCOrA==' 
[INFO] 2023-10-16T16:27:16,484 [ForkJoinPool.commonPool-worker-31] [] [context.SiteContextManager] | File watcher event type: 'DELETE'. File affected: '/home/juanvasquez/crafter/craftercms/crafter-authoring/data/repos/sites/test3/sandbox/.git/next-index-66222.lock'. Hash value: 'none'

Should we consider /.git/ files to be excluded by default from engine watcher? This to avoid potentially filling the logs by some sort of configuration from the users, specially with big sites or any sort of CI/CD. Thank you for your response!

phuongnq · 2023-10-17T09:36:57Z

@sumerjabri We can have a list of ignore path patterns such as:

# Engine file change watcher ignores path patterns
crafter.engine.watcher.ignorePathPatterns=\
  /.git/**

Please let me know if we want to implement this. However, in a normal case, we should only rebuild on the default value including the /config/engine and /scripts folder.

sumerjabri · 2023-10-17T13:36:19Z

@phuongnq let's build the ignore paths feature and default it to /.git as you indicated. It would be great if we can do the logic like this:

Compute what we are asked to watch
Subtract what we are asked to ignore
This way we can have it exclude specific files even in /config

Euquimides · 2023-10-18T17:52:53Z

Hi! Fix verified for Crafter v4.0.8 and v.4.1.2. Thank you! Closing this ticket.

russdanner added enhancement triage Requires triage labels Sep 21, 2023

russdanner added priority: high CI labels Sep 21, 2023

russdanner added this to the Next Release milestone Sep 21, 2023

sumerjabri assigned phuongnq Sep 21, 2023

sumerjabri removed the triage Requires triage label Sep 21, 2023

This was referenced Sep 25, 2023

Add directory watcher dependency #6230

Merged

Add files watcher for rebuild preview craftercms/engine#657

Merged

sumerjabri self-assigned this Sep 25, 2023

sumerjabri removed their assignment Sep 25, 2023

phuongnq mentioned this issue Sep 26, 2023

Preview target rebuild in engine instead of http call craftercms/deployer#395

Merged

sumerjabri added the add to QA label Oct 2, 2023

sumerjabri assigned yacdaniel Oct 2, 2023

This was referenced Oct 5, 2023

Add directory watcher dependency #6238

Merged

Preview target rebuild in engine instead of http call craftercms/deployer#397

Merged

Add files watcher for rebuild preview craftercms/engine#661

Merged

sumerjabri unassigned phuongnq Oct 5, 2023

jmendeza mentioned this issue Oct 6, 2023

Remove incorrect upgrader craftercms/deployer#398

Merged

yacdaniel assigned Euquimides and unassigned yacdaniel Oct 13, 2023

sumerjabri assigned phuongnq and unassigned Euquimides Oct 17, 2023

This was referenced Oct 18, 2023

Add preview rebuild watcher ignore paths craftercms/engine#664

Merged

Add preview rebuild watcher ignore paths craftercms/engine#665

Merged

sumerjabri assigned Euquimides and unassigned phuongnq Oct 18, 2023

Euquimides closed this as completed Oct 18, 2023

sumerjabri mentioned this issue Oct 26, 2023

[deployer][engine] External commits do not trigger the context rebuilds #4616

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[engine] Engine should automatically detect when classes and config are changed and reload the context #6223

[engine] Engine should automatically detect when classes and config are changed and reload the context #6223

russdanner commented Sep 21, 2023 •

edited

russdanner commented Sep 21, 2023 •

edited

phuongnq commented Sep 25, 2023

sumerjabri commented Sep 25, 2023

sumerjabri commented Sep 25, 2023

phuongnq commented Sep 26, 2023

sumerjabri commented Sep 28, 2023

sumerjabri commented Oct 4, 2023

Euquimides commented Oct 16, 2023 •

edited

phuongnq commented Oct 17, 2023

sumerjabri commented Oct 17, 2023

Euquimides commented Oct 18, 2023

[engine] Engine should automatically detect when classes and config are changed and reload the context #6223

[engine] Engine should automatically detect when classes and config are changed and reload the context #6223

Comments

russdanner commented Sep 21, 2023 • edited

Latest version

Duplicates

Is your enhancement request related to a problem? Please describe.

Describe the solution you'd like

russdanner commented Sep 21, 2023 • edited

phuongnq commented Sep 25, 2023

sumerjabri commented Sep 25, 2023

sumerjabri commented Sep 25, 2023

phuongnq commented Sep 26, 2023

sumerjabri commented Sep 28, 2023

sumerjabri commented Oct 4, 2023

Euquimides commented Oct 16, 2023 • edited

phuongnq commented Oct 17, 2023

sumerjabri commented Oct 17, 2023

Euquimides commented Oct 18, 2023

russdanner commented Sep 21, 2023 •

edited

russdanner commented Sep 21, 2023 •

edited

Euquimides commented Oct 16, 2023 •

edited