dmulyalin / ttp

Template Text Parser
MIT License
350 stars 34 forks source link

Seeing error when trying to use ttp library in AWS environment. #54

Closed Arpan198829 closed 3 years ago

Arpan198829 commented 3 years ago

Hi, Thanks for providing this fabulous library in python.

I am using ttp(https://pypi.org/project/ttp/) library in my codebase and running in AWS cloud environment. Its throwing the below error when running in AWS but there is no issue when running in local machine.

_[ERROR] 2021-06-18T15:26:47.950Z 5a2a654b-1356-43ec-86d9-7a52cbc283b7 ttp.lazy_import_functions: failed to save ttp_dict_cache.pickle '[Errno 30] Read-only file system: '/var/task/ttp/ttp_dictcache.pickle''****

I gone through the ttp codebase and found that in "ttp.py" file on line:165 and 166 its trying to write something in "ttp_dict_cache.pickle" file and in AWS cloud environment as there is no such write permission to write in /var so its throwing the above error. Note, the functionality is working fine in AWS environment as well as local environment(Where no issue of updating the .pickle file).

My suggestion would if you can change the log from "log.error" to "log.info or log.warn" then it will not come in the AWS console. I am providing the code snippet in below.

Code snippet:

# try to cache/pickle _ttp_ to a file for future use
try:
    with open(os.path.dirname(__file__) + "/ttp_dict_cache.pickle", "wb") as f:
        pickle.dump(_ttp_, f)
except Exception as e:
    _log.error(
        "ttp.lazy_import_functions: failed to save ttp_dict_cache.pickle '{}'".format(
            e
        )_
    )  

Please let me know if you change it and merge it in master so that I can pull the changed code.

dmulyalin commented 3 years ago

Hi,

Thanks for using TTP, changed log level to warning in 8066009dafdc553f23267d89325b94c61e7fdc23 commit, pushed to master.

Arpan198829 commented 3 years ago

Hi,

Thanks for your help.

AWS Lambda doesn't allow to write in any directory except /tmp and ttp library is unable to write in ttp_dict_cache.pickle file as the file is not under /tmp in AWS environment.

To resolve this issue, is it possible to check for the file under /tmp directory[as we can access /tmp in AWS] first by importing the folder name from environment like os.getenv["ttp pickle file location"]. In this way ttp will create a new .pickle file under /tmp and can use it in any environment. If the environment variable not found then search in specific installed directory where it is currently searching for the .pickle file.

Also, it would be very helpful if you can merge this change in the latest ttp version so that we can get this after doing pip install ttp.

Something like the below highlighted part:

def lazy_import_functions(): """function to collect a list of all files/directories within ttp module, parse .py files using ast and extract information about all functions to cache them within ttp dictionary """ global ttp log.info("ttp.lazy_import_functions: starting functions lazy import")

# try to load previously pickled/cached _ttp_ dictionary if it exists
try:
    **if(os.getenv['ttp pickle file location']):
        path_to_cache = os.getenv['ttp pickle file location'] + "/ttp_dict_cache.pickle"
    else:
        path_to_cache = os.path.dirname(__file__) + "/ttp_dict_cache.pickle"**

    if os.path.isfile(path_to_cache):
        with open(path_to_cache, "rb") as f:
            _ttp_ = pickle.load(f)
        # use unpickled _ttp_ dictionary if it of the same python version
        if _ttp_["python_major_version"] == version_info.major:
            log.info(
                "ttp.lazy_import_functions: loaded _ttp_ dictionary from ttp_dict_cache.pickle"
            )
            return
        # rebuilt _ttp_ dictionary if version is different
        else:
            _ttp_ = {
                "macro": {},
                "python_major_version": version_info.major,
                "global_vars": {},
                "template_obj": {},
                "vars": {},
            }
except Exception as e:
    log.error(
        "ttp.lazy_import_functions: failed load ttp_dict_cache.pickle, loading functions directly, error '{}'".format(
            e
        )
    )
Arpan198829 commented 3 years ago

Hi,

Could you please help us on this issue. Please go through my previous post on issue in AWS Environment and let me know.

Thanks, Arpan

Arpan198829 commented 3 years ago

Hi,

Its really necessary for the AWS environment. Please help me.

Thanks, Arpan

dmulyalin commented 3 years ago

Hi,

Sorry for long no reply. Pushed master branch to version 0.7.2. on pypi.

To resolve this issue, is it possible to check for the file under /tmp directory[as we can access /tmp in AWS] - nah, ttp_dict_pickle is an optional, non mandatory improvement, that allows to save about 40-60ms of time while importing TTP, everything should work normally even without it, just will take 40-60ms longer to load TTP on initial import. Would rather keep it as is and declare this feature does not work on read only systems

Arpan198829 commented 3 years ago

Thanks for your reply.

In our case, execution time is very important. So if we can save 40 to 50 ms then it will be very good for our application. Currently, the .pickle file is being read form the installed directory not from /tmp as below.

    path_to_cache = os.path.dirname(__file__) + "/ttp_dict_cache.pickle"** 

**So, we are requesting you if you add an extra check for the .pickle file to search under the /tmp directory as well. For ex: if(os.path.exists("/tmp/ttp_dict_cache.pickle")): path_to_cache = os.path.dirname(/tmp) + "/ttp_dict_cache.pickle" else: path_to_cache = os.path.dirname(file) + "/ttp_dict_cache.pickle"

In this way, .pickle file will be searched first under /tmp, if not found then it will take it from the installed directory.

It will be very helpful if you do the above change and merge it to the latest ttp version so that we can get this after doing pip install ttp.**

Thanks, Arpan

Arpan198829 commented 3 years ago

Hi,

Could you please help us by doing the above changes in ttp codebase so that it checks the .pickle file in /tmp, if not found then search in the installed dir.

Thanks, Arpan

dmulyalin commented 3 years ago

Hi, will make ttp to check environment variable for pickle file location, no eta at this stage.

dmulyalin commented 3 years ago

Hi, added support for TTP_CACHE_FOLDER environment variable in 7640c41 commit

Arpan198829 commented 3 years ago

Hi,

Sorry for late response.

Thanks a lot providing the workaround. There is no such warning or error after this fix. But, still we need some more help regarding the same issue.

In AWS environment, we need to create the "TTP_CACHE_FOLDER" environment variable and we are creating the lamda function and its dependency environment variables(include "TTP_CACHE_FOLDER") using cloudformation template.

Now, the AWS cloudformation template does not support any environment variable name using "_"(underscore) so it will be very helpful if you change the environment variable name from "TTP_CACHE_FOLDER" to "TTPCACHEFOLDER" in your ttp codebase and merge it in master.

Thanks, Arpan

dmulyalin commented 3 years ago

Hi,

Yeah, why not, you are the only one asking for this feature - renamed TTP_CACHE_FOLDER to TTPCACHEFOLDER and merged to master.

Arpan198829 commented 3 years ago

Hi,

I just tried to explain the issue we were facing due to "TTP_CACHE_FOLDER". Yes, please renamed TTP_CACHE_FOLDER to TTPCACHEFOLDER and merged to master.

Thanks you in advance for your support.

Thanks, Arpan

dmulyalin commented 3 years ago

No problems - renamed TTP_CACHE_FOLDER to TTPCACHEFOLDER and merged to master

dmulyalin commented 3 years ago

FYI - TTPCACHEFOLDER is now part of TTP 0.8.0 release

Arpan198829 commented 3 years ago

Thanks a lot for helping me and merging the code in master.

Thanks, Arpan