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

added local type #18

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

added local type #18

wants to merge 4 commits into from

Conversation

dominik-przybyl-wttech
Copy link
Collaborator

No description provided.

@@ -1,6 +1,6 @@
{
"name": "aem",
"version": "0.1.2-alpha.1718453175+e73fab23",
"version": "0.1.2-alpha.1718453293+5c68305a.dirty",
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

@@ -104,7 +111,7 @@ func (ic *InstanceClient) saveProfileScript() error {
}

func (ic *InstanceClient) configureService() error {
if !ic.data.System.ServiceEnabled || ic.data.Client.Type == "local" {
if !ic.data.System.ServiceEnabled {
Copy link
Collaborator

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

Copy link
Collaborator

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

Copy link
Collaborator

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

Copy link
Collaborator

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

@@ -102,6 +102,7 @@ type System struct {
WorkDir string `pulumi:"work_dir,optional"`
Env map[string]string `pulumi:"env,optional"`
ServiceEnabled bool `pulumi:"service_enabled,optional"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

@@ -55,6 +59,8 @@ public sealed class System

bool? service_enabled,
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

Copy link
Collaborator

@krystian-panek-vmltech krystian-panek-vmltech left a 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),
Copy link
Collaborator

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

Copy link
Collaborator

@krystian-panek-vmltech krystian-panek-vmltech left a 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

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

Successfully merging this pull request may close these issues.

2 participants