Conversation
…s and HK2 bindings
|
Can one of the admins verify this patch? |
takesson
left a comment
takesson
left a comment
There was a problem hiding this comment.
Good, address some commends in the readme directly.
Some comments are require more research while doing the refactoring. I suggest we keep this PR open and keep the discussion in these comment threads.
| - Quarkus suggestion: Map to `@ConfigProperty` in `application.properties`. | ||
|
|
||
| 2. cloudId | ||
| - Type and name: Context: cloudId. ENV: CLOUDID |
There was a problem hiding this comment.
Current logic:
context parameter overrides environment variable, enables multiple workers.
The context parameter should be refactored to @ConfigProperty. Let's drop the ENV support.
| - What it does: Identifies the AWS worker instance. If not set, AWS worker will not start. | ||
| - Quarkus suggestion: Map to `@ConfigProperty`. | ||
|
|
||
| 3. aws.accessKeyId |
There was a problem hiding this comment.
Drop both accessKeyId and secretKey. We will rely entirely on DefaultAwsRegionProviderChain.
This is possible because in CMS 5.3 cloud setup we no longer need separate API keys for multi-worker installations.
| 5. AWS Region | ||
| - Type and name: ENV (AWS default region configuration) | ||
| - What it does: Determines which AWS region is used. And default fallback is eu-west-1. | ||
| - Quarkus suggestion: Use `quarkus.amazon.region` or `@ConfigProperty`. |
There was a problem hiding this comment.
Ok, will see which is needed depending on how the S3 / SFN clients are injected. We can remove the default if it simplifies the implementation.
Important:
When running on EC2 it MUST detect automatically, typically via regionProvider.getRegion()
| - What is does: Defines local filesystem export path. If set, export uses local file system instead of S3. | ||
| - Quarkus suggestion: Map to `@ConfigProperty`. | ||
|
|
||
| 8. APTAPPLICATION |
There was a problem hiding this comment.
APTAPPLICATION must remain an ENV because it is used by Arbortext as well.
| - What it does: Enables S3 transfer acceleration. | ||
| - Quarkus suggestion: Map to `@ConfigProperty`. | ||
|
|
||
| 7. PUBLISH_FS_PATH |
There was a problem hiding this comment.
Should remain an ENV (same name)
| - What it does: Determines which AWS region is used. And default fallback is eu-west-1. | ||
| - Quarkus suggestion: Use `quarkus.amazon.region` or `@ConfigProperty`. | ||
|
|
||
| 6. PUBLISH_S3_ACCELERATED |
There was a problem hiding this comment.
Should remain an ENV (same name)
| 2. region | ||
| - Class / Type: Region. | ||
| - Purpose: Represents the AWS region used by AWS clients. | ||
| - Quarkus suggestion: Inject via `@ConfigProperty` or `@Singleton`. |
There was a problem hiding this comment.
Not sure, depends on how clients are injected. Might be better with @produces
| 5. SfnClient | ||
| - Class / Type: SfnClient. | ||
| - Purpose: AWS Step Functions client | ||
| - Quarkus suggestion: Replace with `@Singleton` or Quarkus AWS SDK client injection. |
| - Purpose: Contains multiple export provider implementations. | ||
| - "fs" --> CmsExportProviderFsSingle (local file export). | ||
| - "s3" --> CmsExportProviderAwsSingle (S3 export). | ||
| - Quarkus suggestion: Inject as a singleton bean with `@Produces`. |
There was a problem hiding this comment.
Yes, produce the whole Map which means we (hopefully) don't have to modify the code using this Map.
4 participants
Created a README file for WorkerApplication to explain all the configs and HK2 bindings. Also with some options on how to convert over to Quarkus.