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

Add elasticsearch support to tugboat. #413

Merged
merged 12 commits into from
Feb 22, 2024
Merged

Conversation

KeyboardCowboy
Copy link
Contributor

@KeyboardCowboy KeyboardCowboy commented Feb 14, 2024

Resolves #387

@KeyboardCowboy KeyboardCowboy self-assigned this Feb 14, 2024
@github-actions github-actions bot temporarily deployed to pantheon-pr-413 February 14, 2024 20:43 Destroyed
@github-actions github-actions bot temporarily deployed to pantheon-pr-413 February 14, 2024 20:57 Destroyed
@github-actions github-actions bot temporarily deployed to pantheon-pr-413 February 14, 2024 22:06 Destroyed
@@ -49,7 +51,7 @@ jobs:
# preview - they should be the same.
- name: Test Generated Files
run: |
cmp -b drainpipe/.tugboat/config.yml .tugboat/config.yml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to find a way to test my changes by generating a more robust config file and testing it against a more complete example. The test runner doesn't seem to be able to locate this file, though, so I'm not sure if I'm doing something wrong or if there's a better way to test my work.

Copy link
Member

Choose a reason for hiding this comment

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

@KeyboardCowboy the way we're currently testing it is by having the Tugboat file generated match the one we're using to create a Tugboat environment - bit of a hack, but unfortunately we can't generate a tugboat configuration on the fly. If you add elastic search to .tugboat/config.yml then that should be fine. Is there anything we can add to verify that elasticsearch has started correctly and is usable by Drupal?

@@ -49,7 +51,7 @@ jobs:
# preview - they should be the same.
- name: Test Generated Files
run: |
cmp -b drainpipe/.tugboat/config.yml .tugboat/config.yml
Copy link
Member

Choose a reason for hiding this comment

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

@KeyboardCowboy the way we're currently testing it is by having the Tugboat file generated match the one we're using to create a Tugboat environment - bit of a hack, but unfortunately we can't generate a tugboat configuration on the fly. If you add elastic search to .tugboat/config.yml then that should be fine. Is there anything we can add to verify that elasticsearch has started correctly and is usable by Drupal?

@github-actions github-actions bot temporarily deployed to pantheon-pr-413 February 15, 2024 15:59 Destroyed
@github-actions github-actions bot temporarily deployed to pantheon-pr-413 February 15, 2024 16:47 Destroyed
@KeyboardCowboy
Copy link
Contributor Author

I adjusted the .tugboat/config.yml file with the redis and elasticsearch entires to match what the build action should be adding, but the test is failing the file comparison on line 12, which appears the be the empty line between the mariadb dependency and the commands. Is there a way to view the file the action is generating to compare the two side-by-side? Otherwise what's the recommended way to test the output?

@deviantintegral deviantintegral added the enhancement New feature or request label Feb 19, 2024
@github-actions github-actions bot temporarily deployed to pantheon-pr-413 February 19, 2024 18:59 Destroyed
@deviantintegral
Copy link
Member

@KeyboardCowboy that's a great question. From what I can tell, since diff also sets an error exit code if it fails we can use that, and get the advantage of not having to replicate this locally.

I pushed up a commit to this PR changing that, and you can see the test error in the CI logs. I'll let you fix it!

@github-actions github-actions bot temporarily deployed to pantheon-pr-413 February 19, 2024 20:05 Destroyed
@KeyboardCowboy
Copy link
Contributor Author

@KeyboardCowboy that's a great question. From what I can tell, since diff also sets an error exit code if it fails we can use that, and get the advantage of not having to replicate this locally.

I pushed up a commit to this PR changing that, and you can see the test error in the CI logs. I'll let you fix it!

Thanks, Andrew. It looks like it's working now. Do I need to do something about the sass/JS compilation error? I'm not sure that has anything to do with my changes.

@justafish
Copy link
Member

If you have a look at the error log you can see it failed at downloading the DDEV images, so you can just re-run that test https://github.com/Lullabot/drainpipe/actions/runs/7964643194/job/21742704685?pr=413

@github-actions github-actions bot temporarily deployed to pantheon-pr-413 February 22, 2024 14:16 Destroyed
@github-actions github-actions bot temporarily deployed to pantheon-pr-413 February 22, 2024 14:23 Destroyed
@justafish justafish merged commit 4ca15cc into main Feb 22, 2024
31 checks passed
@justafish justafish deleted the 387--tugboat-elasticsearch branch February 22, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add elasticsearch as a preconfigured service to Tugboat
3 participants