puppet-elasticsearch icon indicating copy to clipboard operation
puppet-elasticsearch copied to clipboard

Published 20 hours ago • verified publisher icon voxpupuli

Add support for composable index templates

Open NITEMAN opened this issue 3 years ago • 7 comments

Pull Request (PR) description

Add support for composable index templates

This Pull Request (PR) fixes the following issues

NITEMAN avatar Feb 22 '22 11:02 NITEMAN

I don't know why "Puppet 6 - Ubuntu 18.04" acceptance is failing but AFAIK it seems related with elastic_stack module and this PR is working as expected

NITEMAN avatar Mar 01 '22 11:03 NITEMAN

hi @NITEMAN, thanks for the awesome work. The type/provider, puppet and test code look all pretty solid. I made some tiny inline comments but overall this looke quite good.

bastelfreak avatar Mar 01 '22 11:03 bastelfreak

Hi @bastelfreak , thanks to all of you for taking over with this module!

Most code is copied/adapted from other parts of the module so I hope my bits are ok... for me the hard part has come dealing with testing so please double check these parts

NITEMAN avatar Mar 01 '22 12:03 NITEMAN

thanks! looks good to me now. I added a few reviewers that have more knowledge about elasticsearch/ruby.

bastelfreak avatar Mar 01 '22 12:03 bastelfreak

Thanks, I'm now confident enough to try to reroll #1102 and #1103 adding tests

NITEMAN avatar Mar 03 '22 10:03 NITEMAN

Removed some leftover todos, add more tests, and fixed some comments.

Any chances for this to be merged soon?

NITEMAN avatar Apr 13 '22 09:04 NITEMAN

Test fail seems unrelated/accidental, but I've no idea of how to re-run test without force-pushing

NITEMAN avatar Apr 13 '22 09:04 NITEMAN

@smortex since you are a member having write access, all tests seem fine and we would be excited to see #1163, #1164 and this one here make it into the master...please could you help find the missing peace (or help out whats missing?)

thank you very much!

usp-rol avatar Mar 13 '23 14:03 usp-rol

Hey :wave:

Thanks for bringing this up @usp-rol. I was about to merge #1164 and realized that the commits are different in each PR.

@NITEMAN, I would recommend to (check and) force-push the commits referenced in #1164 to their respective branches/PR:

The 3 PR should then list the same commits and GitHub / GitHub Changelog Generator should not be confused when we merge #1164.

Thanks!

smortex avatar Mar 13 '23 17:03 smortex

Hi @smortex

My original plan was to rebase #1163 after #1146 merge... and then doing the same with #1164 , that would fit git / github keeping changes atomic... but I'll try your suggestion tomorrow (never done before something like that)

Best regards!

NITEMAN avatar Mar 13 '23 17:03 NITEMAN

Thank you both @smortex and @NITEMAN so much! We're happy to this see these features in a new release.

usp-rol avatar Mar 14 '23 07:03 usp-rol

@smortex I haven't managed to gess how to "foce-push x into y", what I've done is

  • Rebase new-templates (#1146) into current master (git rebase -i upstream/master)
  • Rebase ilm_policies (#1163) into new-templates (git rebase -i new-templates)
  • Rebase slm_policies (#1164) into ilm_policies (git rebase -i ilm_policies)

That should fix any commit reference issues

But now I'm again stuck on an unrelated CI fail (for me it's quite frustrating), I suppose master itself fails "CI / Puppet / Static validations"

NITEMAN avatar Mar 14 '23 08:03 NITEMAN

hi @NITEMAN , you can regenerate the REFERENCE.md with bundle exec rake reference

bastelfreak avatar Mar 14 '23 15:03 bastelfreak

Many thanks @bastelfreak , both for the comment and for your support on IRC... I hope everything will came together now

NITEMAN avatar Mar 14 '23 17:03 NITEMAN

@smortex , @bastelfreak Everything should be fine right now (all branches rebased updating reference on each commit)

Please let me know if there is something missing/needed

NITEMAN avatar Mar 14 '23 17:03 NITEMAN