search

stratospheric-dev / stratospheric

All you need to know to get your Spring Boot application into production with AWS 🍃
https://stratospheric.dev
Apache License 2.0
262 stars 187 forks source link

Chapter 11 & 12 fixes #278

Closed szantopeter closed 5 months ago

szantopeter commented 6 months ago

Fix issues in localstack and AWS for chapter 11 and 12. See the commit messages for the list of issues

BjoernKW commented 5 months ago

Closing this pull request for now, since there are still open questions and failing tests.

szantopeter commented 5 months ago

Hi @BjoernKW the question was open, because I answered your question and I was waiting for you to accept it my answer. There weren't any failing test either.

BjoernKW commented 5 months ago

Hi @BjoernKW the question was open, because I answered your question and I was waiting for you to accept it my answer. There weren't any failing test either.

@szantopeter Please see this comment of mine as well as @giuliopulina's comment about a failing test:

"What's the exact issue here? We should still be able to use the Spring Boot DevTools."

"I haven't tried, but I think that this change would make AbstractDevIntegrationTest fail"

szantopeter commented 5 months ago

Hi @BjoernKW I answered @giuliopulina 's question too. I think it is based on a misunderstanding. I attach a screen shot of my earlier answer here image

giuliopulina commented 5 months ago

Hi @BjoernKW I answered @giuliopulina 's question too. I think it is based on a misunderstanding. I attach a screen shot of my earlier answer here image

You are right, the integration test AbstractDevIntegrationTest is only on the main application and not on the chapters. Nevertheless, I think that some consistency should be kept (if the chapters' folders are meant to be the steps needed to build the application, I would expect to have the same code between chapters and main application, wherever possible).

szantopeter commented 5 months ago

fair point, if @BjoernKW is willing to reopen this PR and prefers this change, then I am happy to apply these changes to the main application too. One thing is for sure, the sample applications in the chapters are broken for now

BjoernKW commented 5 months ago

fair point, if @BjoernKW is willing to reopen this PR and prefers this change, then I am happy to apply these changes to the main application too. One thing is for sure, the sample applications in the chapters are broken for now

As @rieckpil has mentioned before, continuing to update the "chapter" applications and keeping them all in sync ultimately would be a combinatorial maintenance nightmare. These applications are to be understood as educational stepping stones while the only authoritative - and always up-to-date - application is the main one.

szantopeter commented 5 months ago

@BjoernKW your comment makes sense, I am just not sure what is your intent with the chapter applications that are broken now for multiple reasons and this PR would fix them.