- 
                Notifications
    
You must be signed in to change notification settings  - Fork 1.2k
 
Fix file watcher infinite loops and improve logging for local invoke #8196
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
base: develop
Are you sure you want to change the base?
Conversation
        
          
                samcli/lib/utils/file_observer.py
              
                Outdated
          
        
      | """ | ||
| 
               | 
          ||
| # Ignore patterns for file watching to avoid infinite loops and unnecessary events | ||
| DEFAULT_IGNORE_PATTERNS = [ | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we provide a way for customer to provide customize ignore pattern? If none is provided, we can use the DEFAULT_IGNORE_PATTERNS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this should be the idea when we ship it. I will use this version to debug with customer first
| 
           DO NOT MERGE.  | 
    
The base branch was changed.
| 
           This failing test seems unrelated, but will take a look  | 
    
| 
           also passed on my local  | 
    
| 
           What is the reason that we no longer allow customers to provide a list of files/dirs to be ignored? It seems like that was the original intention.  | 
    
          
 The root cause was actually not that. Originally the assumed root cause was few node lib is constantly changing when function is invoking and therefore triggering our file watcher, but then it turns out it's just the file watcher is somehow lagging and it picks up the filechanges way later than it should be. So the lib changes are done in build, which should be a legit change that should be picked up. For what here is fixing, is in some sam project setup, it might have multiple lambda function sharing a same code dir (like a same file with different function as entrypoint). When we are doing  Instead, providing the   | 
    
| "--no-watch", | ||
| is_flag=True, | ||
| default=False, | ||
| help="Disable file watching. Local code changes will not reset running docker container.", | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my understanding, this is only relevant when using warm_container_initialization_mode, right? That's probably something we should mention here, so it's easy for customers to understand when this matters. (same for the comment on start_lambda/cli.py
| if self._no_watch: | ||
| _function_providers_args[self._containers_mode].extend([False, True]) | ||
| elif self._no_watch: | ||
| _function_providers_args[self._containers_mode].extend([False, False, True]) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the same topic from another comment, if a customer uses --no-watch without using --warm-container-initialization-mode it could be that this _function_providers_args[self._containers_mode] refers to SamFunctionProvider, which doesn't have the extra parameter for no_watch, and it would fail with an uncaught error. We should probably have a validation to not try to do that when it doesn't make sense.
Problem
The AWS SAM CLI file watcher was experiencing infinite loops during local development, particularly with LoopBack applications that create temporary
.sandboxdirectories. This caused:Solution
1. Add --no-watch flag to
local start-apiandlocal start-lambdaThis flag would disable the default behavior where a change in codebase will reset the docker container. User can use this flag to debug a changing local dir, and kill & restart the command to pick up new changes.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.