Melhorias ao código #1
Replies: 8 comments 3 replies
-
|
A correção do poetry funcionou bem: |
Beta Was this translation helpful? Give feedback.
-
|
Acho que o ponto principal do pacote não é a manipulação das variáveis de ambiente, mas obter o token utilizado para authenticação. Acredito que o melhor seja separar essas responsabilidades:
Por exemplo: from decouple import config
# settings.py
class Settings:
def __init__(self, **kwargs):
self.email = kwargs.get('email', config('FOGOCRUZADO_EMAIL'))
# auth
class Auth:
def __init__(self, settings: Settings):
self.settings = settings
@property
def token(self):
# TODO: usar settings.email e settings.pasword para obter o token
....
# service.py (não lembro onde a gente usa isso, então pseudo-código mais do qq outra coisa)
settings = Settings()
auth = Auth(settings)
faz_requisicao(URL, token=auth.token)Essa lógica do final pode ser abstraída numa classe Aí teríamos basicamente, três módulos principais:
Acho que assim podemos matar o Sobre o restante, acho que o Eu só faria mais uma adição: pq numa biblioteca chamada Já que vamos fazer um major bump, vamos renomear as coisas para evitar essa redundância? |
Beta Was this translation helpful? Give feedback.
-
|
Pensando um pouco mais, acho que já que vamos de major bump, dá para ser mais arrojadas. Vamos reescrever as instruções do Por exemplo, acho isso ruim (e é o que está documentado hoje): >>> from crossfire import fogocruzado_signin # desnecessário importar fogo cruzado de fogo cruzado
>>> fogocruzado_signin('[email protected]', 'password') # prática não recomendável colocar credenciais no código https://12factor.net/config
>>> from crossfire import get_fogocruzado # mesma coisa da 1ª linha… e meio desnecessário ter que utilizar dois imports
>>> fogocruzado = get_fogocruzado(state=['RJ']) # nomes muito genéricos (get e nome do pacote)Eu sugeriria algo assim >>> from crossfire import Client # nomes não repetitivos, um único import
>>> client = Client() # na hora de instanciar, podemos passar algum parâmetro CASO não queira usar os valores padrão (por exemplo, credenciais já configuradas nas variáveis de ambiente)
>>> data = client.occurrences(states='RJ') # método mais preciso no nome, e tipagem dinâmica (internamente a gente converte de 'RJ' para ['RJ'])Isso escrevo muito pensando em ir além da PEP8. Aí a gente documenta os casos personalizados… por exemplo, senão for de configurar >>> from crossfire import Client, Settings
>>> settings = Settings(
... email=os.envvar['OUTRO_EMAIL'],
... password=os.envvar['OUTRA_SENHA'],
... )
>>> client = Client(settings)E aí saímos escrevendo testes, refatorando e coisa e tal. |
Beta Was this translation helpful? Give feedback.
-
|
Lendo com mais calma, fico com algumas dúvias:
Qual seira a diferença entre essas opções? Imagino que a primeira demandaria do usuário adicionar suas credenciais e algum arquivo que seria lido ao carregar o módulo e que seria usado para acessar o token. É isso?
Achei legal essa abordagem! Aliás, acabei de aprender que com essa abordagem, podemos deixar as variaveis de ambiente em um .env, podemos mandar por um kwargs e ainda como parẫmetro da função. Legal!
Em resumo, concordo com a abordagem. Nao teria qualquer sugestão de melhorias a fazer... |
Beta Was this translation helpful? Give feedback.
-
|
Configurar variável de ambiente
Módulo de configuração
|
Beta Was this translation helpful? Give feedback.
-
|
Sobre o módulo
Seria o caso de termos um |
Beta Was this translation helpful? Give feedback.
-
Não. Você está confundindo aplicação com configuração. A solução para esse problema que você aponta é (lista não exclusiva):
|
Beta Was this translation helpful? Give feedback.
-
|
Entendo.
E essa forma seria pelo
Entendo que o que vc estava propondo é: se não for configurar o FOGOCRUZADO_EMAIL e FOGOCRUZADO_PASSWORD no Posso e devo estar confundindo tudo. Entendo que como boas prácticas o certo seria separar tais variáveis, mas como aprendi a programar em R, acho normal alguem querer "apenas" instalar o pacote e passar as credenciais por parâmetro sem ter que se preocupar com |
Beta Was this translation helpful? Give feedback.
-
|
Tem suas confusões aí : )
Ou seja, por um lado, podemos fazer um erro super explicativo para quem for utilizar: try:
self.email = config('EMAIL_MAGICO')
except UndefinedValueError:
message =
"O Fogo Cruzado precisa estar autenticado para funcionar, no entanto "
"não encontramos a condiguração com o email a utilizar para essa "
"autenticação. Existem algumas formar de configurar esse email:\n"
"- criando uma variável de ambiente chamada EMAIL_MAGICO\n"
"- incluindo uma linha EMAIL_MAGICO=… no arquivo `.env`\n"
"- incluindo a variável EMAIL_MAGICO no arquivo `settings.ini`\n"
)
raise Exception(message)Mas o que eu acho melhor é a inicialização da classe sobrescrevber a chamado o self.email = kwargs.get('email', config('FOGOCRUZADO_EMAIL'))Nesse caso, primeiro a classe tenta ler os argumentos de inicialização, depois chama o try:
self.email = kwargs.get('EMAIL_MAGICO', config('EMAIL_MAGICO')) # alterei aqui
except UndefinedValueError:
message =
"O Fogo Cruzado precisa estar autenticado para funcionar, no entanto "
"não encontramos a condiguração com o email a utilizar para essa "
"autenticação. Existem algumas formar de configurar esse email:\n"
"- criando uma variável de ambiente chamada EMAIL_MAGICO\n"
"- incluindo uma linha EMAIL_MAGICO=… no arquivo `.env`\n"
"- incluindo a variável EMAIL_MAGICO no arquivo `settings.ini`\n"
"- incializando essa classe com o valor do email, por exemplo Settings(email='…')" # adicionei essa linha
)
raise Exception(message) |
Beta Was this translation helpful? Give feedback.
-
Sim :). Cheguei até a escrever o código. Só não cheguei a escrever uma mensagem nao boa quanto a sua. rs
Perfeito! Contudo, ele só irá sobreescrever caso já exista um
Mas estou entendendo que é isso, mesmo. A ideia é lenvantar o erro e assim garantimos que exista:
Neste caso só removeria a ultima linha da sua mensagem... Sim, isso já estava claro. Mas justamente por ter visto a possibilidade de fazer um acochambramento de forma a permitir que usem a função sem ter Seguirei nessa linha e usarei sua mensajem. Aliás, devo traduzir a mensagem? |
Beta Was this translation helpful? Give feedback.
-
|
Fiz uma pequena alteração na sua mensagem, veja o que acha: |
Beta Was this translation helpful? Give feedback.
-
Este Python package foi criado considerando a versão 1 da API do Fogo Cruzado. Se trata de uma API para acessar aos dados gerados por crowd sourcing sobre tiroteiros em algumas cidades do Brasil.
Recentemente, foi lançada a versão 2 da API e gostaria de atualizar o módulo.
Com isso surgiram dúvidas:
Sobre organização de código:
fogocruzado_signin) que transforma os parâmetros (usuário e senha) em variáveis de ambiente (que serão usadas em outras funções. Mas reparem que não faz osignin, de fato. Começo a suspeitar que além do nome não corresponder com o resultado da função, essa função pode estar emfogocruzado_utils.py.fogocruzado_key, que acessa e retorna a chave de acesso à API obtida pela funçãoget_token_fogocruzadoget_token_fogocruzadoacessa as variáveis de ambiente geradas nofogocruzado_signine, em sendo chaves válidas, salva como variáveis de ambiente (a mesma resgatada pelofogocruzado_key);extract_data_apiusa ofogocruzado_keypara acessar o token e endpoint de ocorrencias de tiroteio. Reparem que ele recebe um parâmetrolink. Isso pelo fato dele ser usado numa função "maior", emget_fogocruzado.py. Falo mais dele logo mais....get_cities_apiusar ofogocruzado_keypara acessar o token e resgatar dados do endpoint cities. Os dados retornados são convertidos emDataFrame.get_states_apisimilar aoget_cities_api, mas dessa vez acessa o endpoint de estados. Os dados retornados são convertidos emDataFrame;Sobre essas cinco funções, tenho algumas dúvidas:
URL?Continuando....
- get_cities.py: É um script usa o
get_citites_apie apenas convertir parte dos dados. Contudo, pelo que vi na documentação, esse dado em questão já não é mais retornado pelo endpoint. Ou sjea, não tem mais motivo de exitir.- e get_fogocruzado.py: essa é a função usada para estruturar a URL do
endpointde ocorrencias e retornorará as ocorrencias de tiroteio em formatoGeoDataFrame. É por ela que fica 'a logica de negócio' dos filtros... o que está, não necessáriamente permanecerá. Pois entendo que algumas restrições serão desconsideradas. Mas reparem que há a possibilidade de filtro por data(s) e cidade(s).Demais Dúvidas
Daí surge a pergunta, que em muito se assemelha à pergunta anterior: como fazer com que o filtro de cidade, possa receber o resultado de um retorno à endpoint de cidades da API E também a uma consulta por nome de cidade?
Faz sentido separar tanto o código, também?
Beta Was this translation helpful? Give feedback.
All reactions