-
Notifications
You must be signed in to change notification settings - Fork 0
added local type #18
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
base: main
Are you sure you want to change the base?
added local type #18
Conversation
| { | ||
| "name": "aem", | ||
| "version": "0.1.2-alpha.1718453175+e73fab23", | ||
| "version": "0.1.2-alpha.1718453293+5c68305a.dirty", |
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.
?
|
|
||
| func (ic *InstanceClient) configureService() error { | ||
| if !ic.data.System.ServiceEnabled || ic.data.Client.Type == "local" { | ||
| if !ic.data.System.ServiceEnabled { |
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.
I don't remeber such bool serviceEnabled in TF provider... why have you introduced it? these providers should not be so different
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.
system service is must have; we want to maintain only the single and correct way of setting up AEM instances... maintaining variations will increase complexity
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.
hmm if serviceEnabled = false then runServiceCommand("start") need be replaced by "sh aemw instance launch" command and the similar thing for stopping
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.
for non-admin usages like on local/dev machine it would be fine
| DataDir string `pulumi:"data_dir,optional"` | ||
| WorkDir string `pulumi:"work_dir,optional"` | ||
| Env map[string]string `pulumi:"env,optional"` | ||
| ServiceEnabled bool `pulumi:"service_enabled,optional"` |
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.
?
|
|
||
| string? service_config, | ||
|
|
||
| bool? service_enabled, |
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.
?
krystian-panek-vmltech
left a comment
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.
serviceEnabled / system service is must-have, not an optional thing
| System: compose.SystemArgs{ | ||
| Data_dir: pulumi.String(composeDir), | ||
| Work_dir: pulumi.String(workDir), | ||
| Service_enabled: pulumi.Bool(false), |
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.
I don't want to have ServiceEnabled here
krystian-panek-vmltech
left a comment
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.
if systemd is optional then starting/stopping instance need to be aligned
No description provided.