search

Admiral-Piett / goaws

AWS (SQS/SNS) Clone for Development testing
MIT License
788 stars 146 forks source link

removed sqs. from hostname #194

Closed kcajmagic closed 5 years ago

kcajmagic commented 5 years ago

With the current implementation, the url (assuming host in the config is goaws) will return sqs.goaws. If the user is running goaws in docker and leveraging container name for networking the current implantation won't work.

kcajmagic commented 5 years ago

I could be wrong and there maybe a better way to leverage docker-compose yaml files.

p4tin commented 5 years ago

The reason the .sqs is there was because that is the url that the real sqs uses - I was figuring you could leverage the /etc/hosts file if you needed the name to work. Usually the queue URL is not used by goaws as you can just connect using the IP address or localhost... But perhaps I am not understanding your particular use case.... Please elaborate...

kcajmagic commented 5 years ago

While it's nice to make the application more like the real sqs. Having the addition hostname can cause headaches for those running the app, since you have to handle the additional hostname. Changing the /etc/hosts can solve this problem, but it doesn't scale well.

p4tin commented 5 years ago

I do not think scaling is important for an application that is strictly for development purposes.

kcajmagic commented 5 years ago

That is fair. However, this is adding extra steps to the consumer of this repo. Since the url is in the response to the api, the consumer should be able to hit that endpoint directly. How it is currently requires extra work somewhere, wether is is through /etc/hosts, setting up a dns server, or client side logic.

For example, if you are running the docker image by itself on port 4100. The initial requests to setup the subscription will be too localhost:4100. The url sqs.localhost:4100 will be in the response payload for confirming the subscription. The client should be using that url and should not have to do extra work. This will not work by itself because dns resolution will fail to resolve the host sqs.localhost

p4tin commented 5 years ago

@kcajmagic you are correct I will merge this and release it.

p4tin commented 5 years ago

@kcajmagic - released @v0.3.0 (docker image also created same version)

kcajmagic commented 5 years ago

@p4tin thank you, this is awesome.