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

Move setting of parameters before loading services #349

Closed
wants to merge 2 commits into from

Conversation

daFish
Copy link

@daFish daFish commented Sep 11, 2024

This change fixes the following issue: When you configure a custom dump_directory, the setting is not passed to services. The extension loads the services before setting the parameters resulting in the default for dump_directory.

Todo:

  • add tests covering the behaviour

Bonus: Remove unused imports from testfile.

@yann-eugone
Copy link
Member

yann-eugone commented Sep 12, 2024

Hello, thank you for this suggestion.

However, I don't see the trouble you are having here.
The parameters and the services can be loaded in a different order, it will not have any effect, because everything is done at compile time.

For instance, here is the parameter registered in a standard project

$ bin/console debug:container --parameter=presta_sitemap.dump_directory
 ------------------------------- ------------- 
  Parameter                       Value        
 ------------------------------- ------------- 
  presta_sitemap.dump_directory   /src/public  
 ------------------------------- ------------- 

And here is what Symfony knows about a service using these parameters

bin/console debug:container presta_sitemap.dump_command --show-arguments

Information for Service "presta_sitemap.dump_command"
=====================================================

 Command to dump the sitemaps to provided directory

 ---------------- -------------------------------------------------- 
  Option           Value                                             
 ---------------- -------------------------------------------------- 
  Service ID       presta_sitemap.dump_command                       
  Class            Presta\SitemapBundle\Command\DumpSitemapsCommand  
  Tags             console.command                                   
                   container.no_preload                              
  Calls            setName                                           
  Public           yes                                               
  Synthetic        no                                                
  Lazy             no                                                
  Shared           yes                                               
  Abstract         no                                                
  Autowired        no                                                
  Autoconfigured   no                                                
  Arguments        Service(router.default)                           
                   Service(presta_sitemap.dumper_default)            
                   /src/public                                       
  Usages           .service_locator.VztzE8J                          
 ---------------- -------------------------------------------------- 

As you can see, the presta_sitemap.dump_directory parameter is injected in the presta_sitemap.dump_command with the appropriate value: /src/public.

@daFish
Copy link
Author

daFish commented Sep 13, 2024

@yann-eugone I stand corrected. This works as expected and must be someting related to my setup. I'll close this PR.

@daFish daFish closed this Sep 13, 2024
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.

2 participants