Add LOG_LEVEL and TERM_CLIENT_CLOSE_TIMEOUT environment variables

[andrey-tech]

Hello, Team!

Please, add options log_level and term_client_close_timeout to supported environment variables as LOG_LEVEL and TERM_CLIENT_CLOSE_TIMEOUT.

[kickster97]

Thanks for the suggestions, we will try to look into a solution to cover more configuration options!

[ArieGato]

Hi,

any progress concerning this issue? We're using the docker image of amqproxy and want to configure these settings throught env vars as well. I could make a contribution, but I'm not familiar with Crystal.

[kickster97]

Hi! We have not had time to get started with this yet and would appreciate the contribution if you have the time, we can guide you in the right direction with crystal so no worries there!

[ArieGato]

So, I cloned the repo to try to get it working on my (windows) machine. I'm following the steps in CONTRIBUTING.md, but I'm running in some issues. First, the line endings of spec/entrypoint.sh appear to be CR LF. When I change them to LF, I get an error when running this command: docker run --rm -it -v $(pwd):/app -w /app --entrypoint bash amqproxy_spec.

LLVM: 15.0.7
Default target: x86_64-pc-linux-gnu
++ crystal spec --order random
Showing last frame. Use --error-trace for full trace.

In src/amqproxy/server.cr:3:1

 3 | require "amq-protocol"
     ^
Error: can't find file 'amq-protocol'

There ends my little adventure. Any pointers?

[kickster97]

Ah cool, have you run shards install? That will install the dependencies so the program recognizes them :)

[ArieGato]

I'm using the existing dockerfile:

amqproxy/spec/Dockerfile

Line 9 in 7958a89

RUN shards install

Maybe I have to build the image from another working folder?

[ArieGato]

I got it working by using the docker compose file. Maybe the CONTRIBUTION.md file needs updating?

I do have another question though. It appears that the config file settings override the environment variables. Is this desired behavior? When working with Docker images it is common that environment variables overrule config files as this is only way to differentiate between environments.

[kickster97]

Good that you got it working! And you are right, CONTRIBUTION.md probably needs an update :)

Its a good question, and I don't think there is a specific reason we let the config file trump the ENV. I have put up a draft for changing that priority so it behaves more as expected.

[ArieGato]

Great! Let me know how it turns out.

I'm working on a proto type of a Config class that handles all configuration. That way we can also add Specs for the class. Is that a good way to go? I'm still struggling a bit, though. I would like to create an immutable object, but its not exactly working like c#, that being my native language :-) I'll figure it out and when I have something I'm confident about, I'll let you know, so you can have a look at it.

[kickster97]

Ok sounds good, let me know :)

[ArieGato]

I have a question about Crystal.

I want to call a private method from a class method, something like this:

The private constructor appears to be called successfully, but the private method is throwing an exception. Is this a construct that is not supported by Crystal?

  class Config

    private def initialize
    end

    private def load_from_options(args)
      # do something
    end
  

    def self.load_with_cli(args, path = "config.ini") : Config
      config = new

      config.load_from_options(args);

      config
    end
  end

[kickster97]

According this issue crystal-lang/crystal#5995 it should be possible? But i tested out your code and it works if you make load_from_options a protected method instead of a private

this is another open issue kind of relagted to this, have not read through both issues super closely but leaving them here if you want to look into them
crystal-lang/crystal#12717

[ArieGato]

I did some playing around and I have two examples. Could you look at them and give me some pointers how to proceed?

a class with getters:
https://github.com/ArieGato/amqproxy/blob/feature/add-config-class/src/amqproxy/config.cr

a record:
https://github.com/ArieGato/amqproxy/blob/feature/add-config-class/src/amqproxy/config_record.cr

thnx