Feature/threading

[krowvin]

Added optional flag -p or --parallel to enable multi-threaded CDA fetch requests. This can drop run times significantly since CDA requests are I/O bound and not CPU bound (i.e. waiting on the database/network).

What

• Use queue/threading and split the CDA logic out for parallel requests
• revert to original run method if the -p flag is not provided
• Add a pytest for fetching using the -p flag and compares against an expected output (set datetime) / Add this to the github workflow
• Split the test directories out by district, thinking more districts/we can create per district tests for active forms
• Move various variables like version, thread count, repgen5 github issues url to the repgen/__init__.py
• Experiment with how to go about looping projects and dynamically setting the variables for form generation

Why

• SWT has a monthly report that takes upwards of 5 minutes to complete. This won't matter as much when things are running headless/on cron/airflow/etc. But during testing having the ability to run tests in parallel for a complete report output in a fraction of the time will be great!

Bumped version to 5.1.0

Issue(s)

• Make HTTP requests async/parallel #30

[DanielTOsborne]

The actual threading code seems mostly fine, aside from one section I added a prior comment to, there's just a lot of unrelated changes and questions.

[DanielTOsborne]

Is this still used? The comment was removed, but the variable itself wasn't. If this is still used, is it threadsafe now?

[krowvin]

The variable is a _ meaning it should be private if such a scope existed for python

The way I did the threads each of the Value classes spawns it's own call for data. This means the scope is limited for threading to each Value class.

This was just an oversight after all those changes and I missed this comment.

[DanielTOsborne]

Why was pytz downgraded?

[krowvin]

I was running the freeze in the wrong environment and this is me reverting those extra changes.

I also noticed there were at least two requirements files in the project. Had to make sure not to overwrite those or get them confused.

Might we work checking to see what the version was prior because the line is not green/red meaning I think that's what it was before? I could be wrong there though

I.e. hasn't been updated since 2022.1 ?

[krowvin]

@jbkolze downloaded the branch and did some testing.

Right out of the gate he realized the threads were not actually doing anything...
From
d8f04d7
Onward I worked on fixing that issue and was able to get the runtimes down from 20seconds to 1-2seconds for 15 threads.

However, there is one looming issue. I spent some time trying to solve it.

This branch is not production ready without this bug fix

tldr; race conditions are hard!

Currently you are able to re-use Value calls, and this seemingly works.

For example if you do:

_value = Value(
        dbtype="radar",
        DBLOC="KEYS",
        DBPAR="Elev",
        DBPTYP="Inst",
        DBINT="1Hour",
        DBDUR="0",
        DBVER="Ccp-Rev",
        DBUNITS="ft",
    )

This will write the values to the form output

Then if you chain another and change just the project for example

_value = Value(
        DBLOC="FGIB"
    )

Then this will also write values the correct values to the form.

So it seems the deepcopy code works across the values.

The problem arises when you want to take the results of some value and feed that into another Value constructor. For example:

_value = Value(
        DBLOC="FGIB"
    )
    
_dates = Value(_value.datatimes(),
        PICTURE="%d%b%Y %K%M",
    ) 

This will end up failing because _value will be in another thread fetching the data and is non-blocking in this new threading branch. So it will continue on with an empty Values property array that then results in no results for .datatimes().

Some thoughts I had on how to deal with this:

1. We switch to async io and determine per value if it is a TIMESERIES type or a COPY type for the Value - If not a TS type we block the request and wait for others (requires end users to be aware of how to structure their forms)
2. We split the string into a third category internally.

• Right now we split into the FORM and the DEF. We could split the DEF into a DEP and INDEP (dependent and independent)
• Then have all the independent values fire first (your timeseries/data calls)
• Followed by all the DEP calls.
  ⚠️ Problem with this method is figuring out how to pass the values after the fact since (afaik) python does not have the concept of promises. Python's threading Event came to mind though.

3. Instead of having the actual data in datatimes() we return the properties of the class and have it refetch values (inefficient)

I'm moving on to the other pressing issues - but will revisit this time willing!

[MikeNeilson]

Okay, so there are 2 ways (well, maybe more but I can only think of 2) to deal with this chicken and egg problem.
1.
Value itself, while it should be thread safe, is independent and doesn't do any threading.... or maybe it is, like you said this type of design is hard
Instead you make a function that takes a list of Value objects and returns them, this function itself waits, but otherwise retrieves/let's retrieve the Values in parallel.

2. Instead of letting the user make a "Value" directly you use a factory method to provide a "Future/Promise" (whatever it's called in Python) and when you get to a portion that has to wait you can get the actual Value.

After typing that, they aren't really mutually exclusive. Using more of a builder pattern (factory method, actual class, etc) we could just initialize the object with the query parameters and another call is required to actually get them.

However, consider that will add some complexity to people getting onboarded. Honestly I think I like the idea of method that takes multiple definitions and when it returns the data is ready. Seems like explaining "set all your data, then do math" is easier to explain than the intricacies of threading issues.

[krowvin]

sing more of a builder pattern (factory method, actual class, etc) we could just initialize the object with the query parameters and another call is required to actually get them.

Seems #21 is right in line with what you suggest here.