Skip to content

Remplazo del sdk de minio por el de s3 oficial#113

Merged
arcosa merged 17 commits into
masterfrom
s3-library
Oct 27, 2025
Merged

Remplazo del sdk de minio por el de s3 oficial#113
arcosa merged 17 commits into
masterfrom
s3-library

Conversation

@JuanMartinP
Copy link
Copy Markdown
Collaborator

Se pasa de usar el sdk minio-py para usar boto3

Base automatically changed from minio-library-functionalities to master October 22, 2025 07:49
Comment on lines -80 to -84
return Minio(
self.endpoint,
self.access_key,
self.secret_key,
secure=self.secure
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Esto haría desaparecer el from minio import Minio y, en cascada, la dependencia de minio en setup.py, ¿no?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Es la intención si, pero como la librería oficial de aws para s3 es más compleja (o tiene una documentación más enrevesada al menos) lo estoy intentando hacer progresivamente.

Comment thread python-lib/test-etl-lib-minio.py Outdated

# declare minioManager
minio_manager = tc.minioManager(endpoint='<minio_endpoint>:<port>',
minio_manager = tc.s3Manager(endpoint='<http/https>://<minio_endpoint>:<port>',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Deberiamos buscar un nombre "neutro".

Al final tanto Minio como S3 son Object Storaraes. Igual osManager, objectStorageManager... no es que me gusten mucho, pero no se me ocurre otro.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

A mi S3 me parecía más correcto porque al final se puede referir a un api estándar de object storages que supuestamente ahora serían todos compatibles con esta librería, pero es cierto que está ligado a AWS porque también llaman s3 a su servicio. No sé si preferimos perder este contexto que da el nombre para tener un nombre genérico, al final también es cierto que con nuestra documentación te debería dar igual si es s3 o no.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

En mi opinion S3 tiene una conotación fuerte de servicio de AWS. Si lo dejamos como s3Manager puede dar la (falsa) impresión de que por debajo tenemos AWS y no una instancia de Minio. De ahí lo de usar un nombre más neutro.

Pero igual estoy sesgado :) @arcosa @AlvaroVega ¿qué opinais vosotros?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A mi tambien me gusta mas la marca blanca "Minio"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

objectStoreManager es buena opción, es como el concepto generico, almacenamiento de objetos (buckets y archivos) que viene a ser la función de S3 y MinIO. Otra opción bastante limpia y que abstrae de relacionarlo con s3 y minio, sería bucketManager al final gestionamos un sistema de archivos en buckets. El concepto de bucket te lleva rápido al tipo de almacenamiento del que estamos hablando.. independiente de que sea a través de s3, minio, etc ..

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

De entre los dos últimos me decanto por objectStoreManager

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

objectStoreManager me parece correcto, lo cambio y hago los ajustes necesarios a la documentación en cuanto termine los tests.

@JuanMartinP JuanMartinP requested a review from fgalan October 24, 2025 12:28
Comment thread python-lib/tc_etl_lib/README.md Outdated
Comment thread python-lib/tc_etl_lib/README.md Outdated
Comment thread python-lib/tc_etl_lib/README.md Outdated
- :param obligatorio `endpoint`: enpoint de acceso a MinIO
- :param obligatorio `access_key`: usuario necesario para hacer login en MinIO
- :param obligatorio `secret_key`: contraseña necesaria para hacer login en MinIO
- :param optional `secure`: flag para indicar si la conexión con MinIO usa https (True) o http (False). Por defecto se considera `True` si se omite el parámetro.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

¿con boto no hay opción de conexión no segura?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Boto no tiene un flag específico pero en el endpoint acepta http y https, así que para mi demo por ejemplo me bastó con poner http y ya.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Es decir, que ya en el URL schema (sea https, sea http) lo hace adecuadamente. Mejor que con la lib de Minio entonces :)

NTC (entendido y todo ok)

Comment thread python-lib/tc_etl_lib/README.md Outdated
JuanMartinP and others added 2 commits October 27, 2025 14:07
Co-authored-by: Fermín Galán Márquez <fgalan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@fgalan fgalan left a comment

Choose a reason for hiding this comment

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

LGTM

Paso bola a los otros revisores. Cuando esté el LGTM de todos, el último que mezcl, pls.

Copy link
Copy Markdown
Collaborator

@AlvaroVega AlvaroVega left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Collaborator

@arcosa arcosa left a comment

Choose a reason for hiding this comment

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

LGTM

@arcosa arcosa merged commit e9de470 into master Oct 27, 2025
5 checks passed
@arcosa arcosa deleted the s3-library branch October 27, 2025 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants