-
Notifications
You must be signed in to change notification settings - Fork 11
Add support for doctrine/annotations 2.0 and remove requirement to doctrine/cache #32
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
Add support for doctrine/annotations 2.0 and remove requirement to doctrine/cache #32
Conversation
69e7c46 to
c5be651
Compare
| - php-version: '7.2' | ||
| elasticsearch-version: '5.6.14' | ||
| lint: false | ||
| symfony-version: '^2.8' |
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.
support for Symfony 2 dropped because of requirement to symfony/cache
f04b5c8 to
7167d69
Compare
| $hasContainerBuildId = $container->hasParameter('container.build_id'); | ||
| if (!$hasContainerBuildId) { | ||
| // the 'container.build_id' is required for `es.cache_engine` system cache which normally can not | ||
| // be constructor inside a compiler pass. This is a workaround to make it work. | ||
| // see also: | ||
| // - https://github.com/symfony/symfony/blob/52a92926f7fed15cdff399c6921100a10e0d6f61/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php#L389 | ||
| // - https://github.com/symfony/symfony/blob/52a92926f7fed15cdff399c6921100a10e0d6f61/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L2322 | ||
| $hasContainerBuildId = true; | ||
| $container->setParameter('container.build_id', hash('crc32', 'Abc123' . time())); | ||
| } | ||
|
|
||
| $collector = $container->get('es.metadata_collector'); | ||
| if (!$hasContainerBuildId) { | ||
| $container->getParameterBag()->remove('container.build_id'); | ||
| } |
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.
This is required when using the cache.system and access it inside the compile time. Another possibility would be to use the cache.app but which does not require build_id. But metadata normally is invalidated between deployments and should fast readable which normally is job of the cache.system and not the cache.app.
You have requested a non-existent parameter "container.build_id".
aef3071 to
adeb229
Compare
| - name: Composer require | ||
| if: ${{ matrix.symfony-version == '^4.4' }} | ||
| # symfony 4.4 not compatible with doctrine/annotations 2.0: https://github.com/symfony/symfony/issues/48717#issuecomment-1359164836 | ||
| run: | | ||
| composer require --no-update doctrine/annotations:^1.10 | ||
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.
Required for: Error: Call to undefined method Doctrine\Common\Annotations\AnnotationRegistry::registerLoader() in PHP 7.3 with Symfony 4.4
0b41a5f to
7bc87fa
Compare
7bc87fa to
11de7a1
Compare
11de7a1 to
6cc7081
Compare
|
The tests are failing because ES7 does not support some features of this bundle. But looks like it the same as on current 5.x branch: #38 |
8817569 to
1f2dc99
Compare
TODO
require atleast
1.13and updateCachedReaderto the withPSRCachedReader: doctrine/annotations@c66f06b.This requires that we move from doctrine/cache to symfony/cache psr cache think metadata can use the symfony system cache.
In this case i completely removed the requirement to
doctrine/cacheand replaced it in all cases with the PSRCacheReader. This is a small BC break which should mostly not have effects on existing systems. Still I would go here with a new Minor release so somebody could still require5.4.*when still want to use doctrine/cache or older annotations version.