Config class

[NickPapONS]

Added functionality to StringProperty, created IntegerProperty, and also Config class. Added unit tests for all StringProperty and IntegerProperty methods, Config unit tests had to be reverted and put on hold until some feedback hopefully helps complete the class.

Almost finished, but I am uncertain on a couple of points and would like some more input, so please have a look at the described lines and see if it makes sense:

In string.py line 30-31:
Not sure what kind of matching process is meant to be done here? This affects how the string property would be validated.

In config.py line 47:
This is where a property is defined and populated from the input dictionary if it is to be of the StringProperty type. What I need help on is what the "value" field should be assigned to. I understand what the value field represents, but don't really see where that is coming from in the input dictionary. (If you need an example of what an input dictionary looks like see here: https://github.com/GSS-Cogs/dp-python-tools/tree/main/dpytools/config )

Would appreciate just a general sanity check on the config class/the from_env method. This was pretty confusing at first but I think the loop that goes through the dictionary and uses setattr() to make attributes dynamically based on the dictionary contents basically does what the ticket specifies. It's not 100% done yet but hopefully another pair of eyes can get it there. Thanks.

Also, not sure why there are so many weird binary files in the Files changed. Looking into it now

[mikeAdamss]

So this is quite a gnarly one so I had to work through it myself. Code below but I need to explain a few things first.

@staticmethod

Is just syntactic sugar, it just associates a function with a class for usibility, so

class MyClass:

       @staticmethod
       def from_env(): -> MyClass:
          ...

is identical in functionality to

MyClass:
   ...
   
  
 # Note - just a function, not a method on the class
def myclass_from_env() -> MyClass
   ...
   

It''s basically loose assocation to make code easier to read (in this case making it clear its the Config class that we're making from env vars).

....which is why... you codes not working, because there's no "self" in a function.

Hopefully the following tweaked version will make more sense.

annotations

That from future import at the top does some magic stuff. Without that you cant reference a class within itself (the -> Config) type hint after .from_env. That's all this import is - allowing that type hint/annotation.

worked example

Like I said, it was easier to just work this through myself, can you run through this and make sure t makes sense to you and come back at me with any questions?

from __future__ import annotations

import os
from typing import Dict, List

from .properties.base import BaseProperty
from .properties.string import StringProperty
from .properties.intproperty import IntegerProperty

class Config:
    
    def __init__(self):
        self._properties_to_validate: List[BaseProperty] = []

    @staticmethod
    def from_env(config_dict: Dict[str, Dict[str, BaseProperty]]) -> Config:

        config = Config()

        for env_var_name, value in config_dict.items():

            value_for_property = os.environ.get(env_var_name, None)
            assert value_for_property is not None, f'Required envionrment value "{env_var_name}" could not be found.'

            if value["class"] == StringProperty:
                if value["kwargs"]:
                    regex = value["kwargs"].get("regex")
                    min_len = value["kwargs"].get("min_len")
                    max_len = value["kwargs"].get("max_len")
                else:
                    regex = None
                    min_len = None
                    max_len = None

                # Best way to determine populating regex, min len, max len
                # if they are none?    
                stringprop = StringProperty(
                    name = value["property"],
                    value = value_for_property,
                    regex = regex,
                    min_len = min_len,
                    max_len = max_len
                )

                prop_name = value["property"]
                setattr(config, prop_name, stringprop)
                config._properties_to_validate.append(stringprop)

            elif value["class"] == IntegerProperty:
                if value["kwargs"]:
                    min_val = value["kwargs"].get("min_val")
                    max_val = value["kwargs"].get("max_val")
                else:
                    min_val = None
                    max_val = None

                intprop = IntegerProperty(
                    name = value["property"],
                    value = value_for_property,
                    min_val = min_val,
                    max_val = max_val
                )

                prop_name = value["property"]
                setattr(config, prop_name, intprop)
                config._properties_to_validate.append(intprop)

            else:
                raise ValueError("Incorrect value type specified for property assignment.")

        return  config


    def assert_valid_config(self):
        """
        Assert that then Config class has the properties that
        provided properties.
        """
        for property in self._properties_to_validate:
            property.type_is_valid()
            property.secondary_validation()

        self._properties_to_validate = []

Take your time to understand that, using a static method for class constructors in this way is one of those python pattern that's really handy when used in just the right circumstances, worth some brain cells.

I might have some comments on the code itself but worth going through the rewrite and any tests first and we'll go from there.

validation

I might change my mind in time, but for now we want a separate validation we can call against the Config class, don't bother validating each property as you construct, aim for a populated Config class with an assert_valid_config method that someone can choose to call.

Hope that all makes sense, have a look and grab me if you want to get into the weeds on any of this.

[mikeAdamss]

coming along nicely. Added a few comments but its around the edges, the meat of this is done and working.

[mikeAdamss]

couple of bits of tidy up, but this is essentially done.

[mikeAdamss]

You'll need to delete all these .pyc files, then commit the fact you deleted them all, that should get them out of repo.

You may need to temporarily unignore them in the gitignore to do it, not sure.

[NickPapONS]

The temporary unignore seems to be what I missed, I'm hoping I did it right this time 😅 let me know if not

[mikeAdamss]

LGTM, well done 👍