The default view mode in CKEditor is configured for Drupal local media but does not apply to Acquia DAM media. This distinction arises because Drupal media utilizes view displays, whereas Acquia DAM media relies on embed codes (similar to image styles used for image media types in Drupal).
I hope this clarifies the your concern.
Requesting review.
rajeshreeputra → created an issue.
Missed to remove queueAssets() method removal from processQueue(). Requesting review.
Requesting review.
The Acquia DAM asset rendering from local functions operates as expected. However, the challenge arises with the Site Studio templates. We will address this issue in Acquia DAM, particularly for the custom templates created by users.
rajeshreeputra → changed the visibility of the branch 1.1.x to hidden.
rajeshreeputra → changed the visibility of the branch 3476376-provide-local-file to hidden.
rajeshreeputra → made their first commit to this issue’s fork.
rajeshreeputra → created an issue.
Hello @ankitv18 - test coverage will be added once the discussion from #7 ✨ Support Drush Command for Bulk Import Feature Active gets resolved.
I would like to propose a discussion regarding the implementation of the commands acquiadam:queue-assets and acquiadam:import-assets. Specifically, we need to consider whether these should be implemented as two separate commands or combined into a single command that handles both queuing and processing of assets.
Additionally, I suggest we introduce an option to limit the asset import process. For instance, we could allow users to specify a limit on the number of assets imported from the queue. An example:
./vendor/bin/drush acquiadam:import-assets --batch-size=50 --item-limit=500
This command would process 10 batches to import a total of 500 assets and then stop, even if there are remaining items in the queue. I believe this feature would enhance the flexibility and usability of the asset import process, especially for users dealing with large datasets.
Looking forward to your thoughts on this!
Updated MR, requesting review.
rajeshreeputra → created an issue.
rajeshreeputra → created an issue.
Requesting review.
rajeshreeputra → changed the visibility of the branch 3415947-whats-the-difference to hidden.
rajeshreeputra → made their first commit to this issue’s fork.
Requesting review.
Updated MR with necessary changes, requesting review.
rajeshreeputra → made their first commit to this issue’s fork.
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
secretvalue 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_valuefrom the key configuration. - When we access the configuration value in CLI:
./vendor/bin/drush cget my_module.settings secretthe returned value should be null. - Even when accessing via configuration export it should be null, in
my_module.settings.ymlfile.
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.