-
Notifications
You must be signed in to change notification settings - Fork 232
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
Support for Multiple domains #327
Support for Multiple domains #327
Conversation
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
A heads up that there is a lot of potential overlap/conflict with #319 . #319 also follows a very similar approach of breaking the domains/HTTP type apart and running them through the process: https://github.com/amplify-education/serverless-domain-manager/pull/319/files#diff-ed009b6b86b017532ef0489c77de5100R148 I think #319 PR could be fixed with a small tweak here to support multiple domains, https://github.com/amplify-education/serverless-domain-manager/pull/319/files#diff-ed009b6b86b017532ef0489c77de5100R243 , but I haven't investigated it thoroughly yet. |
@tehnrd Hmm, just skimming things briefly it might be easy to do since you broke the stuff into an array. Basically a lot of the work was just having things be able to run in a loop, the only secret sauce after that is just being able to have |
Cool cool, I hate seeing all the work in this PR be blocked by mine :-/ but I think that will be the least amount of conflicts in the end. I'll give yours a deeper dive tomorrow and see what good parts we can pull out. |
@tehnrd All good about the work! I would rather just see the change be realized haha, i have no attachments to it :). But i will leave some comments on important places |
Thank you for the PR. Is there an ETA when this will land? For me it is unclear how this works: I want two domains (mydomain.com, mydomain.co.uk) to point to the same websocket api (same stage). How would you specify this in serverless.yml? Maybe an example / explanation in the param description would be helpful. |
@florianbepunkt so i have not looked at this before all those changes were made, i need to look into this again, some larger changes were done unfortunately |
@eunanhardy this is ready to be merged i believe |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
@tehnrd All Tests Passing, No merge conflicts on my end, seems to work like a charm. good work @ConradKurth |
@eunanhardy is there are timeline on getting these changes in? |
Fixes #88
Description of Issue Fixed
Wanted support to be able to specify multiple domains to be created
Changes proposed in this pull request:
Instead of storing the domain inside the ServerlessCustomDomain class, I moved those variables to a new class that would handle the initialization of all those variables.
In the class ServerlessCustomDomain, I made an array of the domains we wish to make an iterate over them to create the multiple domains