Azure-Samples / jmeter-aci-terraform

Scalable cloud load/stress testing pipeline solution with Apache JMeter and Terraform to dynamically provision and destroy the required infrastructure on Azure.
MIT License
120 stars 99 forks source link

Implementation For larger infrastructure #29

Closed hepsi204 closed 4 years ago

hepsi204 commented 4 years ago
ghost commented 4 years ago

CLA assistant check
All CLA requirements met.

allantargino commented 4 years ago

Hello @hepsi204 ! Thank you so much for this wonderful work, congratulations for putting together such great example!

I have some considerations about it. I think we may need to break it into different PRs with different purposes:

What I believe we should take, break and accept:

What I believe we should just add as documentation:

What I believe we shouldn't introduce now:

What I would like to ask you:

I would love your thoughts on these topics! Again, thank you so much! Let's work together to introduce all the great changes you are proposing!

allantargino commented 4 years ago

@hepsi204 I marked it as a Draft PR so we can iterate on it :)

hepsi204 commented 4 years ago

Hi @allantargino ,

Thank you for getting back to me so soon and the kind words. :)

I needed to bounce ideas off you all to decide what to keep/drop hence I added the entire implementation as a separate folder as this is my very first open source PR, so your feedback is perfect.

I also think all the points made were good points.

What are your thoughts on making JMeter slave counts configurable as parameters so users can choose number of slaves to initialise ? - keep/drop ?

I will get started on breaking the work into PR's.

Thanks, Hepsiba Reddy

allantargino commented 4 years ago

Hi @hepsi204! I am sorry it took so long to reply you back now!

I think that having that as a library variable makes the experience easy for me. Do you think that it's best to have it as a parameter inside a template?

allantargino commented 4 years ago

I didn't intend to close this, sorry! I deleted the master branch in favor of the new main one and didn't realize this PR was in draft. But I think most of the code is being iterated on #33 right? Feel free to open this again (and target the main branch).