Skip to content

Minio library functionalities#110

Merged
fgalan merged 31 commits into
masterfrom
minio-library-functionalities
Oct 22, 2025
Merged

Minio library functionalities#110
fgalan merged 31 commits into
masterfrom
minio-library-functionalities

Conversation

@JuanMartinP
Copy link
Copy Markdown
Collaborator

@JuanMartinP JuanMartinP commented Oct 13, 2025

@JuanMartinP JuanMartinP requested a review from fgalan October 16, 2025 11:03
@fgalan
Copy link
Copy Markdown
Contributor

fgalan commented Oct 16, 2025

Comment thread python-lib/tc_etl_lib/setup.py Outdated
'numpy==1.24.4'
'numpy==1.24.4',
'minio==7.2.7',
'pytest-minio-mock==0.4.19'
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.

Si no es una dependencia de producción (es decir, se puede construir el paquete de la libreria y funciona sin esta dependencia) entonces mejor no incluirlo aqúi.

Incluirlo en la GitAction, por aquí https://github.com/telefonicasc/etl-framework/blob/master/.github/workflows/unit-testing.yml#L32

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.

Lo he movido en el commit 882c491

Comment thread python-lib/test-etl-lib-minio.py Outdated
minio_client = minio_manager.initClient()

# Upload test-file.txt to python-test-bucket/output/example.txt
# Important: the bucket must already exist, so
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.

"so" ...

¿quizás ha quedado incompleta la frase?

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.

Si, la frase comentaba que hay que tener el bucket creado antes de subir el fichero, pero escribir eso me hizo pensar que eso se debería gestionar en la librería, así que lo incluí en el método de subir fichero y aparentemente me dejé la frase. La elimino.

@JuanMartinP JuanMartinP requested a review from fgalan October 16, 2025 16:50
Comment thread python-lib/tc_etl_lib/README.md Outdated
Comment on lines +265 to +270
processing_method=print)

# You can define your own custom processing method and use it in the processing_method argument of the getProcessedFile method
def customProcessingMethod(file_chunk):
# code to apply to the chunk of the file or to locally save the file

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.

Creo que sería más ilustrativo usar un método propio directamente que haga una función sencilla (de hecho, puede ser el propio print). Algo de este estilo:

minio_manager.getProcessedFile(minio_client,
                               bucket_name='python-test-bucket',
                               destination_file='/output/example.txt',
                               chunk_size=3,
                               processing_method=process_chunk)

definiendo process_chunk() así:

def process_chunk(file_chunk):
    print(file_chunk)

Comment thread python-lib/tc_etl_lib/README.md Outdated
- :param obligatorio `secret_key`: contraseña necesaria para hacer login en MinIO
- `initClient`: inicializa un cliente de MinIO
- :return: cliente autenticado de MinIO.
- `createBucket`: comprueba si existe el bucket y si no lo crea.
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.

¿Y si ya existe no hace nada o da error?

Comment thread python-lib/tc_etl_lib/README.md Outdated
- `createBucket`: comprueba si existe el bucket y si no lo crea.
- :param obligatorio `client`: cliente de MinIO.
- :param obligatorio `bucket_name`: nombre del bucket a crear.
- `removeBucket`: comprueba si existe el bucket y si existe lo borra.
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.

¿Y si no existe no hace nada o da error?

Comment thread python-lib/tc_etl_lib/README.md Outdated

## Changelog

0.17.0 (October 16th, 2025)
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.

Suggested change
0.17.0 (October 16th, 2025)

De momento no marques cierre de versión (aunque es muy probable que con esta feature cerremos versión, puede que entren más cosas antes).

Comment thread python-lib/tc_etl_lib/README.md Outdated
## Changelog

0.17.0 (October 16th, 2025)
- Add: new class `minioManager` to manage MinIO connection and file processing
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.

Cuando existe issue asociado, incluimos su referencia. Ver como se está haciendo en algún otro caso en el Changelog existente.

Comment thread python-lib/tc_etl_lib/README.md Outdated
- `removeBucket`: comprueba si existe el bucket y si existe lo borra.
- :param obligatorio `client`: cliente de MinIO.
- :param obligatorio `bucket_name`: nombre del bucket a borrar.
- `uploadFile`: sube un fichero a MinIO. Si el bucket al que se sube no existe se crea previamente.
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.

¿Si el fichero ya existe no hace nada o da error?

Comment thread python-lib/tc_etl_lib/README.md Outdated
- :param obligatorio `client`: cliente de MinIO.
- :param obligatorio `bucket_name`: nombre del bucket donde se va a buscar el fichero.
- :param obligatorio `destination_file`: nombre del fichero en MinIO (puede incluir el path SIN el nombre del bucket al inicio).
- :param obligatorio `chunk_size`: tamaño en bytes de cada fragmento del fichero a recuperar.
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.

Igual debería ser opcional, con un chunk_size por defecto razonable.

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.

Imagino que lo más habitual será usar MinIO para ficheros grandes, pongo por defecto por ejemplo 500000 (500kB / 0,5mB)?

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.

No tengo especial criterio, pero 500KB suena bien.

@arcosa @AlvaroVega ¿cómo lo veis?

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.

500Kbs parece razonable para todo tipo de ficheros sin penar mucho el rendimiento.

Comment thread python-lib/tc_etl_lib/README.md Outdated
- `getProcessedFile`: procesa un fichero de MinIO por fragmentos y le aplica a cada fragmento la función provista.
- :param obligatorio `client`: cliente de MinIO.
- :param obligatorio `bucket_name`: nombre del bucket donde se va a buscar el fichero.
- :param obligatorio `destination_file`: nombre del fichero en MinIO (puede incluir el path SIN el nombre del bucket al inicio).
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 realidad el fichero es más bien un origen de datos a procesar, no un destino.

Yo me dejaría de lios y lo llamaría simplemente "file".

- :param obligatorio `destination_file`: nombre del fichero en MinIO (puede incluir el path SIN el nombre del bucket al inicio).
- :param obligatorio `chunk_size`: tamaño en bytes de cada fragmento del fichero a recuperar.
- :param obligatorio `processing_method`: método a aplicar a cada fragmento del fichero.

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.

Estoy viendo que todos los métodos usan client como parámetro. ¿Es realmente necesario? Entiendo que ese cliente está asociado al propio minioManager en sí. En otros manager (cbManager, iotaManager) no se está usando un cliente como parámetro en las invocaciones.

Por otro lado, igual podemos simplificar la vida al usuario y que no tenga que hacer initClient. Que se haga en la propia construcción del manager o al invocar al primer método que lo requiera.

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

# Custom method that writes the file chunks in a CSV (he receives and writes bytes)
def customCSVProcessingMethod(file_chunk):
fichero_procesado = open("salida.csv", "ab")
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.

fichero_procesado

Mejor usar todos los nombres en inglés.

(No sé si habrá más casos como este, revisar)

Comment thread python-lib/tc_etl_lib/tc_etl_lib/minioManager.py Outdated
Comment thread python-lib/tc_etl_lib/tc_etl_lib/minioManager.py Outdated
@JuanMartinP JuanMartinP requested a review from fgalan October 20, 2025 10:11
Comment thread python-lib/tc_etl_lib/README.md Outdated
Comment thread python-lib/tc_etl_lib/README.md Outdated
- :param obligatorio `bucket_name`: nombre del bucket donde se va a buscar el fichero.
- :param obligatorio `file`: nombre del fichero en MinIO (puede incluir el path SIN el nombre del bucket al inicio).
- :param obligatorio `processing_method`: método a aplicar a cada fragmento del fichero.
- :param optional `chunk_size`: tamaño en bytes de cada fragmento del fichero a recuperar.
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.

Especificar el default (similar al comentario anterior).

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.

Cambiado en el commit fe9d4ff

JuanMartinP and others added 2 commits October 20, 2025 15:06
Co-authored-by: Fermín Galán Márquez <fgalan@users.noreply.github.com>
@arcosa
Copy link
Copy Markdown
Collaborator

arcosa commented Oct 20, 2025

Por seguir un poco la misma nomenclarutra que el resto de módulos que componen la librería (auth/cb/iot/minio), renombraría el minioManager.py a minio.py. Ajustando el __init__.py. Y dentro del minio.py ya estaría el manager o otras clases que pudiera necesitar relacionadas con minio.

image

@JuanMartinP
Copy link
Copy Markdown
Collaborator Author

Por seguir un poco la misma nomenclarutra que el resto de módulos que componen la librería (auth/cb/iot/minio), renombraría el minioManager.py a minio.py. Ajustando el __init__.py. Y dentro del minio.py ya estaría el manager o otras clases que pudiera necesitar relacionadas con minio.

image

Cambiado en el commit d985e5a

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 y dejo paso a @fgalan

Comment thread python-lib/tc_etl_lib/README.md
- :param obligatorio: `data`: Datos a enviar. Puede ser una lista de diccionarios o un DataFrame.
- :raises SendBatchError: Se levanta cuando se produce una excepción dentro de `send_http`. Atrapa la excepción original y se guarda y se imprime el índice donde se produjo el error.

- Clase `minioManager`: En esta clase están las funciones relacionadas con la solución de almacenamiento de objetos MinIO.
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.

Un tema de style.

Mirando el style que estamos usando en esta librería, diría que los nombres de los manager van en camelCase (en ese sentido minioManage OK) pero los métodos van en snake_case.

En este caso ajustar

  • create_bucket
  • remove_bucket
  • etc.

Tanto en docu, como en test, como en ejemplos, etc.

(Debería ser relativamente fácil con un "Replace All" en el IDE)

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.

Cambiado en el commit 1c8ec6c

Comment thread python-lib/tc_etl_lib/README.md Outdated
Comment thread python-lib/tc_etl_lib/README.md Outdated
JuanMartinP and others added 3 commits October 21, 2025 16:37
Co-authored-by: Fermín Galán Márquez <fgalan@users.noreply.github.com>
@JuanMartinP JuanMartinP requested a review from fgalan October 22, 2025 07:00
Comment thread python-lib/tc_etl_lib/README.md Outdated
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

@fgalan fgalan merged commit add632b into master Oct 22, 2025
5 checks passed
@fgalan fgalan deleted the minio-library-functionalities branch October 22, 2025 07:49
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