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

[DOCS-7067] ACS Transform override case #934

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

Amy-Hyland
Copy link
Contributor

@Amy-Hyland Amy-Hyland commented Feb 22, 2023

⚠️ Content is missing for ACS 7.3: content-services/7.3/develop/repo-ext-points/content-transformers-renditions.md

@Amy-Hyland Amy-Hyland requested a review from a team as a code owner February 22, 2023 19:25
mstrankowski
mstrankowski previously approved these changes Mar 17, 2023
@@ -218,6 +218,9 @@ changed by resetting the following Alfresco global property.
local.transform.pipeline.config.dir=shared/classes/alfresco/extension/transform/pipelines
```

Local transform-core-aio docker image might need a [newer version of image and approach](#adding-pipelines-and-failover-transforms-to-a-t-engine)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, sorry, I dont know whats going on here. Does this make sense? The link text doesnt make sense in the sentence does it? And I'm not sure about the use of Might.

It also links to a section that isnt called: newer version of image and approach.

Cheers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I understand how it doesn't quite make sense. This is a line directly from the developer for this special case. I can change the link text, and hopefully everyone will accept the changes, and it will be easier to understand from a user's perspective.

Copy link
Contributor

@mstrankowski mstrankowski Mar 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the comment here

In general if someone has issues with overriding configs, we should refer them to the 3.0.0 version of alfresco-transform-core, which is backwards compatible, since it is waaay easier in the new version, but it isn't like it's impossible to do in other cases (aside from AIO), just harder.

Copy link
Contributor

@oliveralfresco2018 oliveralfresco2018 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor suggestions :-)

…ers-renditions.md


This is a great edit of the provided documentation I copied from the public docs for acs-packaging https://github.com/Alfresco/acs-packaging/blob/master/docs/custom-transforms-and-renditions.md?plain=1

Co-authored-by: Oliver Stanley <[email protected]>
@Amy-Hyland Amy-Hyland requested a review from mstrankowski March 30, 2023 21:09
@Amy-Hyland Amy-Hyland requested a review from mstrankowski April 17, 2023 20:06
Copy link
Contributor

@oliveralfresco2018 oliveralfresco2018 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment added.

@@ -218,8 +218,7 @@ changed by resetting the following Alfresco global property.
local.transform.pipeline.config.dir=shared/classes/alfresco/extension/transform/pipelines
```

Local transform-core-aio docker image might need a [newer version of image and approach](#adding-pipelines-and-failover-transforms-to-a-t-engine)
with providing config directly to the T-Engine.
Direct override is only possible in 3.0.0 version of alfresco-transform-core and higher. To override local transform-core-aio T-Engine configuration, the docker image needs a newer version of the image and follow this approach providing configuration directly to the T-Engine: [Adding pipelines and failover transforms to a T-engine](#adding-pipelines-and-failover-transforms-to-a-t-engine).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Amy-Hyland I think this makes more sense. What do you think?
Direct override is only possible in the 3.0.0 version of alfresco-transform-core and above. To override the local transform-core-aio T-Engine configuration, the docker image must use a newer version of the image. Follow this approach and provide the configuration directly to the T-Engine: Adding pipelines and failover transforms to a T-engine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mstrankowski Does Oliver's suggestion still convey the technical details correctly? Can I incorporate his changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants