search

aerospike / aerospike-client-rust

Rust client for the Aerospike database
https://www.aerospike.com/
Other
82 stars 28 forks source link

Adds a functional Makefile #96

Open brianbruggeman opened 3 years ago

brianbruggeman commented 3 years ago

Adds a simple Makefile to standardize development

Addresses #95.

jonas32 commented 3 years ago

Looks fine. I just found one little point i would try to avoid. You hardcoded the ports into the config. What do you think about making them as env vars with a default value?

In addition, do you see any way to use the normal not docker release of the server? Maybe building the server from a custom dockerfile. While integrating Expressions, i figured out that the docker Image is a few days behind sometimes. Im not sure if this is a big deal but that made me spend some extra hours back then.

brianbruggeman commented 3 years ago

I had actually thought about adding a port variable initially, but then rejected it because:

  1. I think it would be good to test against 3 ports rather than 1.
  2. If I wanted to add three ports, I think I'd need 3 port environment variables
  3. I think AEROSPIKE_HOSTS would probably need to reflect the choice of port environment variables
  4. The ports would really only be for the local system because the container would always use the default of 300x

At that point, I wondered if there was really much sense in adding a port variable.

That said I do want to make another update to the Makefile, so if you think there's enough of a good reason despite the above 4, I'm not opposed to adding it.

jonas32 commented 3 years ago

No you are right. I didnt think about just overwriting the whole variable. Thats perfectly fine.

brianbruggeman commented 3 years ago

@jonas32

I took your thoughts and made several port environment variables using: https://www.aerospike.com/docs/operations/configure/network/

At this point you can now override where they are resolved locally.