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

ah_other config split applies to all Acquia Hosted environments #4392

Closed
dpremo-ul opened this issue Aug 19, 2021 · 2 comments · Fixed by #4396
Closed

ah_other config split applies to all Acquia Hosted environments #4392

dpremo-ul opened this issue Aug 19, 2021 · 2 comments · Fixed by #4396
Labels
Bug Something isn't working

Comments

@dpremo-ul
Copy link

Under BLT 12, the ah_other config split is enabled on all Acquia Hosted environments. Under BLT 11, the ah_other config split was enabled on Acquia Hosted environments that are not dev, stage, or prod.

(We have an sbox environment that is separate from dev, stage, and prod. Under BLT 11, the ah_other split was enabled ONLY on our sbox environment. To get around the issue in BLT 12, I effectively made the ah_split empty.)

This can be tested by creating an ah_other config split on an Acquia hosted site running Drupal 8 and BLT 11; and creating an ah_other config split on an Acquia Hosted site running Drupal 9 and BLT 12. Under BLT 11, the ah_other split WILL NOT BE enabled on dev, stage, and prod. Under BLT 12, the ah_other split WILL BE enabled on dev, stage, and prod.

Behavior under BLT 12 is not consistent with documentation here:

https://docs.acquia.com/blt/developer/config-split/

See "Any Acquia environment not listed here".

The logic change occurs in this commit:

affea1d

If you look on the left side (before the commit), starting on line 68, and keep in mind our dev environment, you can see that isAhEnv() returns true, so $split is set to 'ah_other'. But on line 72, isDevEnv() returns true, so $split is set to 'dev'. When we get to line 84, the value of $split is 'dev', so only the dev split is enabled.

On the right side (after the commit), the logic is different, and all splits that "return true" (i know, i'm skipping some steps here https://github.com/acquia/blt/blob/main/src/Robo/Common/EnvironmentDetector.php#L284 ) are enabled. So for my dev environment, isAhEnv() returns true, so the ah_other split is enabled. isDevEnv() returns true, so the dev split is enabled.

The logic has changed.

The name 'ah_other' implies Acquia Hosted environments other than dev, stage, and prod. But in current BLT 12, that's not what it is. It's more like 'ah_all'.

I would suggest the commit above be reverted. I don't think it accomplishes the ask from #4176, although I admit I haven't researched #4176.

@dpremo-ul dpremo-ul added the Bug Something isn't working label Aug 19, 2021
danepowell added a commit to danepowell/blt that referenced this issue Sep 8, 2021
@danepowell
Copy link
Contributor

I agree this is a regression. Can you test #4396 ?

danepowell added a commit that referenced this issue Sep 10, 2021
…4396)

* DX-4625: Fix #4392: [config_split] ah_other shouldn't apply to dev/stage/prod

* stupid colon
@dpremo-ul
Copy link
Author

@danepowell I have tested this PR by using this patch in our composer.json : https://patch-diff.githubusercontent.com/raw/acquia/blt/pull/4396.patch

It looks good. The ah_other split is enabled in our 4th environment. (Ours is called sbox.) And it's disabled in our dev environment. We still need to push it up to test/stage and prod, but so far, so good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants