-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENG-1129: aixplain sdk caching functions #324
Conversation
aixplain/enums/function.py
Outdated
cache_data = {"timestamp": time.time(), "functions": {"enum": {name: value for name, value in functions.__members__.items()}, "input_output": convert_sets(functions_input_output)}} | ||
temp_file = CACHE_FILE + ".tmp" | ||
try: | ||
os.makedirs(os.path.dirname(CACHE_FILE), exist_ok=True); json_str = json.dumps(cache_data); logging.info(f"Serialized JSON size: {len(json_str)} bytes") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why everything in the same line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need to serialize the JSON in bytes? Couldn't you save it using:
import json
with open(CACHE, "w") as f:
json.dump(dictionary, f, indent=4)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I used dump it cut off abruptly for some reason, I changed to serialize and then right, that worked and the json files looked more readable too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please investigate this deeply. The outcome is already a JSON as you can see here. My guess is that there is not need to change anything.
aixplain/enums/function.py
Outdated
|
||
def convert_lists_to_sets(obj): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to do this conversion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a problem with parsing as I mentioned, I thought this might be related because JSON doesn’t support sets, so I changed the sets to lists when saving. I didn’t test it without this conversion after fixing the issue though. I will test removing the conversion, and if everything works fine without it, I’ll clean it up and remove this function.
aixplain/enums/function.py
Outdated
import json | ||
import os | ||
|
||
CACHE_FILE = ".aixplain_cache/functions.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add these constants into aixplain.utils.config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to add the CACHE_FILE into the right constant place.
aixplain/enums/language.py
Outdated
os.makedirs(os.path.dirname(CACHE_FILE), exist_ok=True) | ||
with open(temp_file, "w") as f: | ||
json.dump(cache_data, f) | ||
f.flush() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need to flush?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added flush because the code was cutting off the JSON file while writing, and I thought it might help. flush makes sure everything in the program’s write buffer is sent to the file right away as far as I understand, so the file is fully written before anything else happens. Again this was part of my debugging process to ensure the data was saved correctly. I'll check out if it works properly without it, but out of curiosity isn't is usually recommended to have something like this when writing to external files regardless if the function is not running into any issues? Just to make sure that the sequence of actions runs smoothly
aixplain/enums/language.py
Outdated
CACHE_DURATION = timedelta(hours=24) | ||
|
||
|
||
def save_to_cache(languages): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to create a generic save_to_cache
file that could be used across all files.
aixplain/enums/cache_utils.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add this file in this folder: https://github.com/aixplain/aiXplain/tree/main/aixplain/utils
aixplain/enums/function.py
Outdated
import json | ||
import os | ||
|
||
CACHE_FILE = ".aixplain_cache/functions.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to add the CACHE_FILE into the right constant place.
No description provided.