rajeshreeputra → made their first commit to this issue’s fork.
Requesting review.
rajeshreeputra → created an issue.
rajeshreeputra → created an issue.
rajeshreeputra → made their first commit to this issue’s fork.
rajeshreeputra → made their first commit to this issue’s fork.
rajeshreeputra → created an issue.
Hello @byrond,
You are correct — version_id
and external_id
are not required during the migration process. After migration, the cron in the
Acquia DAM →
module should handle updating version_id
and external_id
regularly and automatically.
Thank you for bringing this up!
rajeshreeputra → changed the visibility of the branch 3527397-allow-subcategories-bulk-import to hidden.
Merged!
rajeshreeputra → made their first commit to this issue’s fork.
Merged!
Merged!
rajeshreeputra → created an issue.
Updated MR with database operations instead of media entity resave, resulting in reduced CPU overhead and improved memory efficiency. This change has led to faster execution times, which should significantly enhance the migration process.
Please review the changes and share your feedback.
Fixed, created MR, requesting review.
The code is needed, if but need to move below the queue create item call.
If the code is removed then the assets will always show the media item status
as outdated
.
Most probably issue introduced because of the AvailabilityPublishingStatusCompare.php.
The AvailabilityPublishingStatusCompare field is calling $this->assetUpdateChecker->checkAssets($values->_entity)
for every single row in the media library view during the render() method.
The Problem
N+1 Performance Issue:
- Media library loads: 12 assets per page
- Each asset triggers:
checkAssets()
call in render() - Result: 12 individual API calls to Acquia DAM
Why this happens
- The
render()
method is called for every row when the view is displayed.
Solution
- Remove the "Availability/Publishing status" field from Media library.
rajeshreeputra → made their first commit to this issue’s fork.
rajeshreeputra → made their first commit to this issue’s fork.
@rlhawk this need to cherry pick into 4.0.x branch as well.
Major Performance Improvements
- Chunked Processing: Instead of loading all media items at once, the code now processes them in smaller batches of 100 items.
- Proper Batch API Usage: The batch operations are now structured to handle each chunk separately, preventing memory exhaustion and timeouts.
- Improved Progress Tracking: Added detailed progress tracking using Drupal's state API, making the process more robust and providing better user feedback.
- Optimized Resource Usage: Service instances like time and current user are now retrieved once at the beginning of each batch operation rather than for each media item.
Implementation Details
- Added new batch operation methods (initializeMediaBatch, batchUpdateMediaItems, finalizeMediaBatch) to handle different phases of the migration process.
- Implemented paged database queries to fetch only the media items needed for each batch operation.
- Added proper progress tracking that persists between batch operations.
- Maintained all the existing functionality while making the process much more efficient.
Expected Results
With these changes, the migration should:
- Complete without timing out.
- Use significantly less memory.
- Provide better progress information to users.
- Likely reduce the total processing time.
rajeshreeputra → made their first commit to this issue’s fork.
rajeshreeputra → created an issue.
Closing.
Removed #3483550 from list as it depends on #3306506 & #3524922
Keeping this intact instead created separate issue for dropping support of Drupal core below 10.3 version.
💬
Drop support of Drupal core below 10.3 version.
Active
rajeshreeputra → created an issue. See original summary → .
Update ticket to Drop support of Drupal core version below 10.3.
Removed 📌 Modernize the Drush commands Needs review from list as it depends on 📌 Remove support for Drupal < 9.1 Needs review .
@ptmkenny, we should update the title, description and MR accordingly to support Drupal core >=10.3 version in 4.x version.
Hello @ptmkenny,
Why is the list an ordered list? Are the issues dependent on one another? (I'm guessing no because some of the later issues have already been committed.)
The issues are not dependent on one another, and it is not necessary for the list to be ordered. We can remove the serial number column from the table.
The big question is should Key drop support for Drupal 8 and/or 9.
The decision to drop this support has already been considered, and it will be removed in the next major version. For more details, please refer to the discussions in the key channel here.
If support for Drupal 8/9 is dropped, I think we should not just "require Drupal 10" but actually take the opportunity to modernize the code and use some of the new features of Drupal 10.
I agree that we should not only require Drupal 10 but also take the opportunity to modernize the code and utilize some of the new features available in Drupal 10. This will be taken into account as part of the next major release, but I will defer to @rlhawk regarding the decision to drop support for Drupal 8 and/or 9 in the 1.x or 2.x.
Requesting review.
rajeshreeputra → made their first commit to this issue’s fork.
rajeshreeputra → made their first commit to this issue’s fork.
Need to resolve the conflits.
Merged!
rajeshreeputra → made their first commit to this issue’s fork.
This is fixed as part of 💬 Fix PHPCS warnings Active
Sorry for the confusion caused by previous comment.
I would like to share a use case to clarify the expected behavior regarding configuration overrides with the Key module.
For example, let's say we have the following configuration:
my_module.settings:
key1: value1
secret: secret_value
- We override the
secret
value using the Key module. - When we access the configuration value in code as:
$config = \Drupal::config('my_module.settings')->get('secret');
the returned value should be the overridden secret from the Key module, i.e.,secret_value
from the key configuration. - When we access the configuration value in CLI:
./vendor/bin/drush cget my_module.settings secret
the returned value should be null. - Even when accessing via configuration export it should be null, in
my_module.settings.yml
file.
requesting review.
rajeshreeputra → made their first commit to this issue’s fork.
But should $config = \Drupal::config('my_module.settings')->get('my_setting'); give me my setting value when this module is in place?
This should not give the value, as value in config now cleared by the override.
Or should I use $config = \Drupal::service('key.repository')->getKey('my_key')->getKeyValue();?
This should give the value from the key.
I think we can make things a bit clearer for users by updating the label.
Right now, "Clear overridden value" might leave some people unsure about what it actually does. I suggest changing it to "Clear overridden value from the configuration item." This way, it’s more specific and helps users understand exactly what will happen when they click it.
rajeshreeputra → made their first commit to this issue’s fork.
rajeshreeputra → changed the visibility of the branch 2972436-ability-to-map to hidden.
Setting this to needs Work.
Agree with @lind101; this requires significant changes and a thorough refactor.
As unable to reproduce with latest dev of key and dependencies mentioned in description, will close the issue as it is functioning as designed. Please feel free to open a new issue if the problem persists.
Resolved conflict and rebased.
Incorporated feedback, hence setting to needs review.
Please review.
Based on the ongoing discussions in the referenced issues, will proceed with the exact ask of this issue without introducing new validations.
Requesting review.
Agree with you that this will introduce compatibility issues and should be implemented only in a new major version.
Sure @cmlara, will start work and discussion on 🐛 KeyProviderInterface Active .
Thank you @mxr576,
This is kinda great learning while working on this issue about choosing between memory cache
and cache bin
.
It's all about asking the right questions on how sensitive and secure the data needs to be. We need to think, "Is this data okay to just be around for a bit to keep it safe or stored for longer?".
Memory cache is fine for quick stuff, but if you need it saved, then cache bin is there, just need to be cautious about where the data might end up.
So yeah, it’s all about figuring out what’s most important for the situation, and then choose accordingly.
💬 Fix PHPCS warnings Active is merged!
Added return type to all interfaces to discuss and finalise accordingly. Then will add return type to all methods in classes.
Cleanup completed, requesting review.
Referencing existing issue to address PHPStan related warnings.
Implemented cache bin in latest commit. Requesting review.
Please review.
Rebased and added to 🌱 Planned for 1.21 version release Active .
📌 Update README.md as per README.md format Needs review and 📌 Fix eslint test Active now merged, added 📌 Modernize the Drush commands Needs review as ready for review.
Requesting review.
📌 Fix deprecation warning. Active is merged!
rajeshreeputra → changed the visibility of the branch 3300224-key-file-not to hidden.
With the understanding how the public://
and private://
stream wrappers function with S3FS. Will close the MR!6(keeping for future reference) and create new one.
Added Reference to release plan 1.21 version.
Updated MR with example as stated in #2, requesting review.
I have not yet utilized the s3fs module in Drupal, but I will give it a try. Then accordingly update here.
rajeshreeputra → changed the visibility of the branch 2985590-static-cache-does to hidden.
rajeshreeputra → made their first commit to this issue’s fork.
Hello @cmlara, updated to work with realpath, stream wrapper and remote urls. Could you please validate with s3 once. Will keep this in needs work status.