Skip to content

Conversation

@avara1986
Copy link
Member

Based on issue #75:

  • Flask configuration will be under pyms block with the key config
  • Services will be under pyms block with the key services

These changes correspond to making the PyMS configuration more readable.

the configuration changed from:

pyms:
  requests:
    data: ""
  swagger:
    path: ""
    file: "swagger.yaml"
my-ms:
  DEBUG: true

to

pyms:
  services:
    requests:
      data: ""
    swagger:
      path: ""
      file: "swagger.yaml"
  config:
    DEBUG: true
    TESTING: false
  • DEPRECATEDS:
    • Env var CONFIGMAP_SERVICE: Configuration block is no longer setted. It will be a constant config
    • The initialization of class Microservice no need the parameter service. Changed from Microservice(service="my-ms", path=__file__,) to Microservice(path=__file__,)

@coveralls
Copy link

coveralls commented Dec 13, 2019

Coverage Status

Coverage increased (+0.007%) to 99.688% when pulling bc6f16e on feature/standarizet_config into b48232f on master.

@alexppg
Copy link
Member

alexppg commented Dec 14, 2019

Se asume que un servicio está activo solo si está declarado? Le ves sentido a que el requisito para activar un servicio sea que tenga la clave enabled: true? Tipo:

pyms:
  services:
    requests:
      enabled: true
      data: ""
    swagger:
      enabled: false
      path: ""
      file: "swagger.yaml"

De esta forma seríá más fácil jugar con las configuraciones y hacer cambios en caliente. En vez de andar comentando muchas líneas, solo cambias un booleano.

from pyms.flask.app import Microservice

ms = Microservice(service="my-ms", path=__file__)
ms = Microservice(path=__file__)
Copy link
Member

Choose a reason for hiding this comment

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

Como es que ya no hace falta? Por otro lado, el path para que se usa?

Copy link
Member Author

Choose a reason for hiding this comment

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

Service servía para definir el nombre del bloque de configuración, por defecto era:

pyms:
  requests:
    data: ""
  swagger:
    path: ""
    file: "swagger.yaml"
ms:
  DEBUG: true

Y dependiendo del nombre que le pongas en la configuración funcionaba así Microservice(service="mi-bloque-de-configuracion", path=__file__)

pyms:
  requests:
    data: ""
  swagger:
    path: ""
    file: "swagger.yaml"
mi-bloque-de-configuracion: # <!--
  DEBUG: true

Al estar ahora todo bajo pyms tiene menos sentido que sea configurable el nombre. Mejor la constante "config"

Copy link
Member

Choose a reason for hiding this comment

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

Si, pero cual es la variable que define el nombre del microservicio? Pregunto por que eso se usa para meterle la etiqueta service con su nombre en las métricas (y supongo que se usará en más lados).

Copy link
Member Author

@avara1986 avara1986 Dec 14, 2019

Choose a reason for hiding this comment

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

Si lo dices por el PR #72
Sería

pyms:
  services:
    tracer:
      component_name: "loquesea"

De la configuración de Flask no hay una variable "oficial" que lo defina: https://flask.palletsprojects.com/en/1.1.x/config/. La que usamos normalmente es la de "app_name" dentro del bloque "pyms.config"

@avara1986
Copy link
Member Author

Se asume que un servicio está activo solo si está declarado? Le ves sentido a que el requisito para activar un servicio sea que tenga la clave enabled: true? Tipo:

pyms:
  services:
    requests:
      enabled: true
      data: ""
    swagger:
      enabled: false
      path: ""
      file: "swagger.yaml"

De esta forma seríá más fácil jugar con las configuraciones y hacer cambios en caliente. En vez de andar comentando muchas líneas, solo cambias un booleano.

Actualizado @alexppg para que admita enabled, pero por retrocompatibilidad si no se declara la keyword "enabled" se asume que si está activo para no dar lugar a confusión

@alexppg
Copy link
Member

alexppg commented Dec 14, 2019

Nice!

@avara1986 avara1986 requested a review from alexppg December 14, 2019 12:20
@avara1986 avara1986 merged commit c3b63f8 into master Dec 15, 2019
@alexppg
Copy link
Member

alexppg commented Dec 15, 2019

Ya está mergeado, pero FYI lo he probado y todo bien, mola! @avara1986

@avara1986 avara1986 deleted the feature/standarizet_config branch January 22, 2020 16:55
@avara1986 avara1986 linked an issue Mar 30, 2020 that may be closed by this pull request
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.

Find a way to standarize the configuration file

5 participants