Skip to content
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

Fission benchmark #56

Open
wants to merge 52 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
3a8dd2a
add fission class
mdamek Jun 6, 2020
78605d1
add run fission script
mdamek Jun 6, 2020
c8c1012
add init and lint
mdamek Jun 6, 2020
5443c4a
wip
mdamek Jun 6, 2020
25f3f0e
add create function
mdamek Jun 6, 2020
fd24ea3
Add making request
mdamek Jun 6, 2020
d4f2687
linting
mdamek Jun 6, 2020
a1edfae
[fission_start_work] Fission k8s deployment written using python scripts
xylini Jun 18, 2020
337314c
edit gitignore
mdamek Jun 20, 2020
42de6c7
manage fission start config
Jun 21, 2020
3f51a5b
add minio
mdamek Jun 20, 2020
6148f76
add packing
mdamek Jun 21, 2020
eb93174
deploy fission function
mdamek Jun 21, 2020
e6e6286
new benchmark
mdamek Jun 21, 2020
d6e9894
cleanup and fix cache error
mdamek Jun 26, 2020
3caaa12
remove unnecessary benchmark
mdamek Jun 26, 2020
341dae3
fix package code directory
mdamek Jun 26, 2020
f434a2b
add handler
mdamek Jun 29, 2020
954c5a4
delpoy function
mdamek Jun 30, 2020
68afc9d
add request invoke
mdamek Jun 30, 2020
460ca18
add requirements handling
mdamek Jun 30, 2020
77004ff
extend handler
mdamek Jul 1, 2020
0ddaded
fix bugs with shutdown
mdamek Jul 1, 2020
d0775cd
[feature_fission] cold start and some comments added
xylini Jul 1, 2020
7d451e9
add logging
mdamek Jul 1, 2020
335984b
Merge branch 'feature_fission' of https://github.com/mcopik/serverles…
mdamek Jul 1, 2020
a6415e1
cold_start fix
mdamek Jul 1, 2020
e88c255
linting
mdamek Jul 1, 2020
4276602
add minio usage
mdamek Jul 2, 2020
25de6ea
lint wrappers
mdamek Jul 2, 2020
d6894c6
lint fission
mdamek Jul 3, 2020
15922f7
s3 comment fix
mdamek Jul 3, 2020
5e816bf
ignore missing types
mdamek Jul 3, 2020
f013a68
[feature_fission] nodejs handler added
xylini Jul 3, 2020
5ad80aa
[feature_fission] space after coma
xylini Jul 3, 2020
36c03a6
add readme
mdamek Jul 3, 2020
9e1cd1c
fix pr 7 comments
mdamek Jul 7, 2020
f31b192
linting
mdamek Jul 7, 2020
9933975
[feature_fission] fission installation through install.py
xylini Jul 14, 2020
16ecf04
Merge branch 'feature_fission' of https://github.com/mcopik/serverles…
xylini Jul 14, 2020
4a4403b
[feature_fission] minio storage added to fission nodejs
xylini Jul 14, 2020
6846a47
Merge branch 'master' into feature_fission
mcopik Sep 23, 2022
4a83600
Merge branch 'master' into feature_fission
mcopik Sep 22, 2023
b3818fd
Merge remote-tracking branch 'origin/master' into feature_fission
mcopik Jun 14, 2024
d732d64
[fission] Linting
mcopik Jun 14, 2024
c0ffa55
WORKING
prajinkhadka Jul 18, 2024
501f2c6
WIP -> Main Functionality is working. Need to optimize the code and r…
prajinkhadka Aug 6, 2024
9106337
WIP -> Main Functionality is working. Need to optimize the code and r…
prajinkhadka Aug 6, 2024
25d98dd
WIP -> Pass Minio storage config as env variables.
prajinkhadka Aug 6, 2024
fcf22cc
WIP -> Do not change funcion.py anywhere
prajinkhadka Aug 6, 2024
af2ca47
Fission Completed Testing on Python3.8
prajinkhadka Aug 9, 2024
46a6f07
Fission Implementation complted.
prajinkhadka Aug 12, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ sebs-*
# cache
cache

# ide
.vscode
# IntelliJ IDEA files
.idea
*.iml
31 changes: 31 additions & 0 deletions benchmarks/wrappers/fission/nodejs/handler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
const path = require('path'), fs = require('fs');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For all wrappers, we need to compare them against newest implementation in OpenWhisk/Knative - there have been changes. In particular, storage triggers received few bugfixes.


exports.handler = async function(context) {
var body = JSON.stringify(context.request.body);
var unbody = JSON.parse(body);
var func = require('./function/function');
var begin = Date.now()/1000;
var start = process.hrtime();
var ret = await func.handler(unbody);
var elapsed = process.hrtime(start);
var end = Date.now()/1000;
var micro = elapsed[1] / 1e3 + elapsed[0] * 1e6;
var is_cold = false;
var fname = path.join('/tmp','cold_run');
if(!fs.existsSync(fname)) {
is_cold = true;
fs.closeSync(fs.openSync(fname, 'w'));
}

return {
status: 200,
body: JSON.stringify({
begin,
end,
compute_time: micro,
results_time: 0,
result: ret,
is_cold
})
};
};
Comment on lines +3 to +31
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding error handling.

The function currently lacks error handling. Consider wrapping the logic in a try-catch block to handle potential errors gracefully.

exports.handler = async function(context) {
+  try {
    var body = JSON.stringify(context.request.body);
    var unbody = JSON.parse(body);
    var func = require('./function/function');
    var begin = Date.now()/1000;
    var start = process.hrtime();
    var ret = await func.handler(unbody);
    var elapsed = process.hrtime(start);
    var end = Date.now()/1000;
    var micro = elapsed[1] / 1e3 + elapsed[0] * 1e6;
    var is_cold = false;
    var fname = path.join('/tmp','cold_run');
    if(!fs.existsSync(fname)) {
        is_cold = true;
        fs.closeSync(fs.openSync(fname, 'w'));
    }

    return {
        status: 200,
        body: JSON.stringify({
            begin,
            end,
            compute_time: micro,
            results_time: 0,
            result: ret,
            is_cold
        })
    };
+  } catch (error) {
+    return {
+      status: 500,
+      body: JSON.stringify({ error: error.message })
+    };
+  }
};
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
exports.handler = async function(context) {
var body = JSON.stringify(context.request.body);
var unbody = JSON.parse(body);
var func = require('./function/function');
var begin = Date.now()/1000;
var start = process.hrtime();
var ret = await func.handler(unbody);
var elapsed = process.hrtime(start);
var end = Date.now()/1000;
var micro = elapsed[1] / 1e3 + elapsed[0] * 1e6;
var is_cold = false;
var fname = path.join('/tmp','cold_run');
if(!fs.existsSync(fname)) {
is_cold = true;
fs.closeSync(fs.openSync(fname, 'w'));
}
return {
status: 200,
body: JSON.stringify({
begin,
end,
compute_time: micro,
results_time: 0,
result: ret,
is_cold
})
};
};
exports.handler = async function(context) {
try {
var body = JSON.stringify(context.request.body);
var unbody = JSON.parse(body);
var func = require('./function/function');
var begin = Date.now()/1000;
var start = process.hrtime();
var ret = await func.handler(unbody);
var elapsed = process.hrtime(start);
var end = Date.now()/1000;
var micro = elapsed[1] / 1e3 + elapsed[0] * 1e6;
var is_cold = false;
var fname = path.join('/tmp','cold_run');
if(!fs.existsSync(fname)) {
is_cold = true;
fs.closeSync(fs.openSync(fname, 'w'));
}
return {
status: 200,
body: JSON.stringify({
begin,
end,
compute_time: micro,
results_time: 0,
result: ret,
is_cold
})
};
} catch (error) {
return {
status: 500,
body: JSON.stringify({ error: error.message })
};
}
};

63 changes: 63 additions & 0 deletions benchmarks/wrappers/fission/nodejs/storage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@

const minio = require('minio'),
uuid = require('uuid'),
util = require('util'),
stream = require('stream'),
fs = require('fs');

class minio_storage {

constructor() {
let minioConfig = JSON.parse(fs.readFileSync('minioConfig.json'));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason why we are using a file here and not envs, like in Python?

let address = minioConfig["url"];
let access_key = minioConfig["access_key"];
let secret_key = minioConfig["secret_key"];

this.client = new minio.Client(
{
endPoint: address.split(':')[0],
port: parseInt(address.split(':')[1], 10),
accessKey: access_key,
secretKey: secret_key,
useSSL: false
}
);
Comment on lines +11 to +24
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle potential file read and parse exceptions.

The file read and JSON parse operations should be wrapped in a try-catch block to handle potential errors.

-	let minioConfig = JSON.parse(fs.readFileSync('minioConfig.json'));
+	let minioConfig;
+	try {
+	    minioConfig = JSON.parse(fs.readFileSync('minioConfig.json'));
+	} catch (e) {
+	    console.error(`Failed to read or parse minioConfig.json: ${e}`);
+	    throw e;
+	}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let minioConfig = JSON.parse(fs.readFileSync('minioConfig.json'));
let address = minioConfig["url"];
let access_key = minioConfig["access_key"];
let secret_key = minioConfig["secret_key"];
this.client = new minio.Client(
{
endPoint: address.split(':')[0],
port: parseInt(address.split(':')[1], 10),
accessKey: access_key,
secretKey: secret_key,
useSSL: false
}
);
let minioConfig;
try {
minioConfig = JSON.parse(fs.readFileSync('minioConfig.json'));
} catch (e) {
console.error(`Failed to read or parse minioConfig.json: ${e}`);
throw e;
}
let address = minioConfig["url"];
let access_key = minioConfig["access_key"];
let secret_key = minioConfig["secret_key"];
this.client = new minio.Client(
{
endPoint: address.split(':')[0],
port: parseInt(address.split(':')[1], 10),
accessKey: access_key,
secretKey: secret_key,
useSSL: false
}
);

}

unique_name(file) {
let [name, extension] = file.split('.');
let uuid_name = uuid.v4().split('-')[0];
return util.format('%s.%s.%s', name, uuid_name, extension);
}

upload(bucket, file, filepath) {
let uniqueName = this.unique_name(file);
return [uniqueName, this.client.fPutObject(bucket, uniqueName, filepath)];
};

download(bucket, file, filepath) {
return this.client.fGetObject(bucket, file, filepath);
};

uploadStream(bucket, file) {
var write_stream = new stream.PassThrough();
let uniqueName = this.unique_name(file);
let promise = this.client.putObject(bucket, uniqueName, write_stream, write_stream.size);
return [write_stream, promise, uniqueName];
};

downloadStream(bucket, file) {
var read_stream = new stream.PassThrough();
return this.client.getObject(bucket, file);
};

static get_instance() {
if(!this.instance) {
this.instance = new storage();
}
return this.instance;
Comment on lines +55 to +58
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using this in a static context.

Using this in a static context can be confusing. Use the class name instead.

-	if(!this.instance) {
-	    this.instance = new storage();
+	if(!minio_storage.instance) {
+	    minio_storage.instance = new storage();
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if(!this.instance) {
this.instance = new storage();
}
return this.instance;
if(!minio_storage.instance) {
minio_storage.instance = new storage();
}
return minio_storage.instance;
Tools
Biome

[error] 55-55: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 56-56: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 58-58: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

}


};
exports.storage = minio_storage;
51 changes: 51 additions & 0 deletions benchmarks/wrappers/fission/python/__main__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import logging
import datetime
import os

import minio
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused import.

The minio import is unused and should be removed.

-	import minio
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import minio
Tools
Ruff

5-5: minio imported but unused

Remove unused import: minio

(F401)


def main(args):
logging.getLogger().setLevel(logging.INFO)
begin = datetime.datetime.now()
args['request-id'] = os.getenv('__OW_ACTIVATION_ID')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test this? This looks like a copy-paste from OpenWhisk

args['income-timestamp'] = begin.timestamp()

for arg in ["MINIO_STORAGE_CONNECTION_URL", "MINIO_STORAGE_ACCESS_KEY", "MINIO_STORAGE_SECRET_KEY"]:
os.environ[arg] = args[arg]
del args[arg]

try:
from function import function
ret = function.handler(args)
Comment on lines +17 to +19
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle potential import exceptions.

The import of function should be wrapped in a try-except block to handle potential import errors.

-	from function import function
+	try:
+	    from function import function
+	except ImportError as e:
+	    logging.error(f"Failed to import function: {e}")
+	    return {
+	        "begin": begin.strftime("%s.%f"),
+	        "end": datetime.datetime.now().strftime("%s.%f"),
+	        "request_id": os.getenv('__OW_ACTIVATION_ID'),
+	        "results_time": (datetime.datetime.now() - begin) / datetime.timedelta(microseconds=1),
+	        "result": f"Error - import failed! Reason: {e}"
+	    }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try:
from function import function
ret = function.handler(args)
try:
from function import function
except ImportError as e:
logging.error(f"Failed to import function: {e}")
return {
"begin": begin.strftime("%s.%f"),
"end": datetime.datetime.now().strftime("%s.%f"),
"request_id": os.getenv('__OW_ACTIVATION_ID'),
"results_time": (datetime.datetime.now() - begin) / datetime.timedelta(microseconds=1),
"result": f"Error - import failed! Reason: {e}"
}
ret = function.handler(args)

end = datetime.datetime.now()
logging.info("Function result: {}".format(ret))
log_data = {"result": ret["result"]}
if "measurement" in ret:
log_data["measurement"] = ret["measurement"]

results_time = (end - begin) / datetime.timedelta(microseconds=1)

is_cold = False
fname = "cold_run"
if not os.path.exists(fname):
is_cold = True
open(fname, "a").close()
Comment on lines +30 to +32
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplify cold start check.

The cold start check can be simplified using a single line.

-	if not os.path.exists(fname):
-	    is_cold = True
-	    open(fname, "a").close()
+	is_cold = not os.path.exists(fname)
+	if is_cold:
+	    open(fname, "a").close()
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if not os.path.exists(fname):
is_cold = True
open(fname, "a").close()
is_cold = not os.path.exists(fname)
if is_cold:
open(fname, "a").close()


return {
"begin": begin.strftime("%s.%f"),
"end": end.strftime("%s.%f"),
"request_id": os.getenv('__OW_ACTIVATION_ID'),
"results_time": results_time,
"is_cold": is_cold,
"result": log_data,
}
except Exception as e:
end = datetime.datetime.now()
results_time = (end - begin) / datetime.timedelta(microseconds=1)
return {
"begin": begin.strftime("%s.%f"),
"end": end.strftime("%s.%f"),
"request_id": os.getenv('__OW_ACTIVATION_ID'),
"results_time": results_time,
"result": f"Error - invocation failed! Reason: {e}"
}
67 changes: 67 additions & 0 deletions benchmarks/wrappers/fission/python/handler.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import datetime, io, json, os, sys, uuid

from flask import request
from flask import current_app

# Add current directory to allow location of packages
sys.path.append(os.path.join(os.path.dirname(__file__), '.python_packages/lib/site-packages'))


def handler():
current_app.logger.info("Received request")
event = request.json
income_timestamp = datetime.datetime.now().timestamp()

req_id = str(uuid.uuid4())
event['income-timestamp'] = income_timestamp
begin = datetime.datetime.now()
from function import function
ret = function.handler(event)
end = datetime.datetime.now()

log_data = {
'output': ret['result']
}
if 'measurement' in ret:
log_data['measurement'] = ret['measurement']
if 'logs' in event:
log_data['time'] = (end - begin) / datetime.timedelta(microseconds=1)
results_begin = datetime.datetime.now()
from function import storage
storage_inst = storage.storage.get_instance()
b = event.get('logs').get('bucket')
storage_inst.upload_stream(b, '{}.json'.format(req_id),
io.BytesIO(json.dumps(log_data).encode('utf-8')))
results_end = datetime.datetime.now()
results_time = (results_end - results_begin) / datetime.timedelta(microseconds=1)
else:
results_time = 0

# cold test
is_cold = False
fname = os.path.join('/tmp', 'cold_run')
if not os.path.exists(fname):
is_cold = True
container_id = str(uuid.uuid4())[0:8]
with open(fname, 'a') as f:
f.write(container_id)
else:
with open(fname, 'r') as f:
container_id = f.read()

cold_start_var = ""
if "cold_start" in os.environ:
cold_start_var = os.environ["cold_start"]
Comment on lines +53 to +54
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use capitalized environment variable COLD_START.

Environment variables are conventionally capitalized. Replace cold_start with COLD_START to follow this convention.

-    if "cold_start" in os.environ:
-        cold_start_var = os.environ["cold_start"]
+    if "COLD_START" in os.environ:
+        cold_start_var = os.environ["COLD_START"]
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if "cold_start" in os.environ:
cold_start_var = os.environ["cold_start"]
if "COLD_START" in os.environ:
cold_start_var = os.environ["COLD_START"]
Tools
Ruff

54-54: Use capitalized environment variable COLD_START instead of cold_start

Replace cold_start with COLD_START

(SIM112)


return {
'statusCode': 200,
'begin': begin.strftime('%s.%f'),
'end': end.strftime('%s.%f'),
'results_time': results_time,
'is_cold': is_cold,
'result': log_data,
'request_id': req_id,
'cold_start_var': cold_start_var,
'container_id': container_id,
}

14 changes: 14 additions & 0 deletions benchmarks/wrappers/fission/python/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from distutils.core import setup
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using setuptools instead of distutils.

setuptools is more commonly used and offers more features and flexibility compared to distutils.

- from distutils.core import setup
+ from setuptools import setup
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
from distutils.core import setup
from setuptools import setup

from glob import glob
from pkg_resources import parse_requirements

with open('requirements.txt') as f:
requirements = [str(r) for r in parse_requirements(f)]

setup(
name='function',
install_requires=requirements,
packages=['function'],
package_dir={'function': '.'},
package_data={'function': glob('**', recursive=True)},
)
150 changes: 150 additions & 0 deletions benchmarks/wrappers/fission/python/storage.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
import os
xylini marked this conversation as resolved.
Show resolved Hide resolved
import uuid
import json
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused import.

The import statement for json is unused and should be removed to clean up the code.

- import json
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import json
Tools
Ruff

3-3: json imported but unused

Remove unused import: json

(F401)

import minio
import logging


class storage:
instance = None
client = None

def __init__(self):
try:
"""
Minio does not allow another way of configuring timeout for connection.
The rest of configuration is copied from source code of Minio.
"""
import urllib3
from datetime import timedelta

timeout = timedelta(seconds=1).seconds

mgr = urllib3.PoolManager(
timeout=urllib3.util.Timeout(connect=timeout, read=timeout),
maxsize=10,
retries=urllib3.Retry(
total=5, backoff_factor=0.2, status_forcelist=[500, 502, 503, 504]
)
)
print("WE get from os")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary

print(os.getenv("MINIO_STORAGE_CONNECTION_URL"))
Comment on lines +30 to +31
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove debugging print statements.

The print statements used for debugging should be removed or replaced with logging for better control over output.

- print("WE get from os")
- print(os.getenv("MINIO_STORAGE_CONNECTION_URL"))
+ logging.debug("Minio connection URL: %s", os.getenv("MINIO_STORAGE_CONNECTION_URL"))

Committable suggestion was skipped due to low confidence.

self.client = minio.Minio(
os.getenv("MINIO_STORAGE_CONNECTION_URL"),
access_key=os.getenv("MINIO_STORAGE_ACCESS_KEY"),
secret_key=os.getenv("MINIO_STORAGE_SECRET_KEY"),
secure=False,
http_client=mgr
)
except Exception as e:
logging.info(e)
raise e
Comment on lines +12 to +41
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improve exception handling and logging.

The current exception handling logs the exception and re-raises it. Consider using logging.error for better visibility of errors and adding more context to the log message.

- logging.info(e)
+ logging.error("Failed to initialize Minio client: %s", e)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def __init__(self):
try:
"""
Minio does not allow another way of configuring timeout for connection.
The rest of configuration is copied from source code of Minio.
"""
import urllib3
from datetime import timedelta
timeout = timedelta(seconds=1).seconds
mgr = urllib3.PoolManager(
timeout=urllib3.util.Timeout(connect=timeout, read=timeout),
maxsize=10,
retries=urllib3.Retry(
total=5, backoff_factor=0.2, status_forcelist=[500, 502, 503, 504]
)
)
print("WE get from os")
print(os.getenv("MINIO_STORAGE_CONNECTION_URL"))
self.client = minio.Minio(
os.getenv("MINIO_STORAGE_CONNECTION_URL"),
access_key=os.getenv("MINIO_STORAGE_ACCESS_KEY"),
secret_key=os.getenv("MINIO_STORAGE_SECRET_KEY"),
secure=False,
http_client=mgr
)
except Exception as e:
logging.info(e)
raise e
def __init__(self):
try:
"""
Minio does not allow another way of configuring timeout for connection.
The rest of configuration is copied from source code of Minio.
"""
import urllib3
from datetime import timedelta
timeout = timedelta(seconds=1).seconds
mgr = urllib3.PoolManager(
timeout=urllib3.util.Timeout(connect=timeout, read=timeout),
maxsize=10,
retries=urllib3.Retry(
total=5, backoff_factor=0.2, status_forcelist=[500, 502, 503, 504]
)
)
print("WE get from os")
print(os.getenv("MINIO_STORAGE_CONNECTION_URL"))
self.client = minio.Minio(
os.getenv("MINIO_STORAGE_CONNECTION_URL"),
access_key=os.getenv("MINIO_STORAGE_ACCESS_KEY"),
secret_key=os.getenv("MINIO_STORAGE_SECRET_KEY"),
secure=False,
http_client=mgr
)
except Exception as e:
logging.error("Failed to initialize Minio client: %s", e)
raise e


@staticmethod
def unique_name(name):
name, extension = os.path.splitext(name)
return '{name}.{random}{extension}'.format(
name=name,
extension=extension,
random=str(uuid.uuid4()).split('-')[0]
)


def upload(self, bucket, file, filepath):
key_name = storage.unique_name(file)
self.client.fput_object(bucket, key_name, filepath)
return key_name

def download(self, bucket, file, filepath):
self.client.fget_object(bucket, file, filepath)

def download_directory(self, bucket, prefix, path):
objects = self.client.list_objects(bucket, prefix, recursive=True)
for obj in objects:
file_name = obj.object_name
self.download(bucket, file_name, os.path.join(path, file_name))

def upload_stream(self, bucket, file, bytes_data):
key_name = storage.unique_name(file)
self.client.put_object(
bucket, key_name, bytes_data, bytes_data.getbuffer().nbytes
)
return key_name

def download_stream(self, bucket, file):
data = self.client.get_object(bucket, file)
return data.read()

@staticmethod
def get_instance():
if storage.instance is None:
storage.instance = storage()
return storage.instance








# import os
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove?

# import uuid
# import json
# import minio
# from flask import current_app
#
#
# class storage:
# instance = None
# client = None
#
# def __init__(self):
# file = open(os.path.join(os.path.dirname(__file__), "minioConfig.json"), "r")
# minioConfig = json.load(file)
# try:
# self.client = minio.Minio(
# minioConfig["url"],
# access_key=minioConfig["access_key"],
# secret_key=minioConfig["secret_key"],
# secure=False,
# )
# except Exception as e:
# current_app.logger.info(e)
#
# @staticmethod
# def unique_name(name):
# name, extension = name.split(".")
# return "{name}.{random}.{extension}".format(
# name=name, extension=extension, random=str(uuid.uuid4()).split("-")[0]
# )
#
# def upload(self, bucket, file, filepath):
# key_name = storage.unique_name(file)
# self.client.fput_object(bucket, key_name, filepath)
# return key_name
#
# def download(self, bucket, file, filepath):
# self.client.fget_object(bucket, file, filepath)
#
# def download_directory(self, bucket, prefix, path):
# objects = self.client.list_objects_v2(bucket, prefix, recursive=True)
# for obj in objects:
# file_name = obj.object_name
# self.download(bucket, file_name, os.path.join(path, file_name))
#
# def upload_stream(self, bucket, file, bytes_data):
# key_name = storage.unique_name(file)
# self.client.put_object(
# bucket, key_name, bytes_data, bytes_data.getbuffer().nbytes
# )
# return key_name
#
# def download_stream(self, bucket, file):
# data = self.client.get_object(bucket, file)
# return data.read()
#
# def get_instance():
# if storage.instance is None:
# storage.instance = storage()
# return storage.instance
Loading