Mudança de parâmetros e funcionamento da função check_results: output para jsonlines

[labanca]

@fjuniorr

A proposta é alterar as funções de teste de:

check_valores_qdd_fiscal <- function(base_qdd_fiscal, acoes_planejamento, stop_on_failure = FALSE, output = FALSE) {
    key <- c("uo_cod", "programa_cod", "acao_cod", "funcao_cod", "subfuncao_cod", "iag_cod")

    [...]

    check_result(df, report, stop_on_failure = stop_on_failure, output = output)
}

Para:

check_valores_qdd_fiscal <- function(base_qdd_fiscal, acoes_planejamento, stop_on_failure = FALSE, output = FALSE, output_tojson = FALSE, msg_template = "") {
    key <- c("uo_cod", "programa_cod", "acao_cod", "funcao_cod", "subfuncao_cod", "iag_cod")

    [...] 

    check_result(df, report, stop_on_failure = stop_on_failure, output = output, output_tojson = output_tojson, msg_template = msg_template)
}

O novo parâmetro output_tojson define se será escrito ou não a saída em um arquivo jsonlines.

O parâmetro msg_template que antes era usada para escrever as mensagens de erro fora da funções de teste:

msg <- "Funcional programática {programa_cod}.{acao_cod}.{funcao_cod}.{subfuncao_cod} da UO {uo_cod} com valor na base QDD Fiscal ({vlr_loa_desp}) diferente da base Açoes Planejamento do SIGPLAN ({vr_meta_orcamentaria_ano0})"

glue_data(check$fail, msg)

Mas que agora seria um parâmetro de cada função de teste e passada ao final delas para a função check_result:

for (i in seq_len(nrow(fail))) {
    log_entry <- list(
      message = glue_data(fail[i, ], msg_template),
      level = "error",
      timestamp = Sys.time(),
      valid = valid
    )
  writeLines(jsonlite::toJSON(log_entry, auto_unbox = TRUE), con)

O commit com a proposta está em 2da19a2dcc9cfe3b8401e2c7ec3b7c487dc47bec. Antes de proceder nessa linha, gostaria de validar essas alterações, até pelo fato de que todos os testes terão de ser alterados em função disso (e se isso é algo desejado).

[fjuniorr]

@labanca porque vc achou importante criar um novo issue no lugar de usar o https://github.com/splor-mg/checks-planejamento/issues/49?

[labanca]

@fjuniorr surgiu algo estranho tentando implementar a lógica discutida na reunião de terça-feira. Eu havia entendido que:

1. Somente haveria um parâmetro para definir a escrita em json. Ele seria NULL para não criar o json ou uma string que seria ao mesmo tempo a indicação que o arquivo json deveria ser escrito e seu caminho. Chamei esse parâmetro de json_outfile.
2. A lógica usaria o operador %||%
3. log_level seria um parametro da função check_results com valor padrão "ERROR".
4. msg_template seria definido dentro do corpo das funções de teste, "funções de fora", e passado para check_results em todos os casos.

Eu cheguei no seguinte código:

check_result <- function(df, report, status = "ok", stop_on_failure, output, summary = NULL,
                         json_outfile = NULL, log_level = "ERROR", 
                         msg_template = "Foram encontrados erros no teste.") {

  check_summary <- validate::summary(report)

  [...]

  output_json_env = {tmp <- Sys.getenv("LOG_FILE", unset = NA); if (is.na(tmp)) NULL else tmp}

  # prioritizes the environment variable
  json_filename <- output_json_env %||% json_outfile

  # Write failure messages as a JSON Lines file 
  if (!is.null(json_outfile) && !valid) {

    con <- file(json_filename, open = "a", encoding = "UTF-8")  
    for (i in seq_len(nrow(fail))) {
      log_entry <- list(
        log_level = log_level,
        timestamp = Sys.time(),
        message = glue_data(fail[i, ], msg_template),
        valid = valid
      )
      writeLines(jsonlite::toJSON(log_entry, auto_unbox = TRUE), con)
    }
    close(con)
  }

  result
}

Quantos aos items 3 e 4 sem problemas. A msg_template está com um valor filler até serem definidas posteriormente.

Nos items 1 e 2 a lógica ficou um pouco estranha. Pois o parâmetro único para escrita do json fazia o uso da variável de ambiente ficar estranha.

No caso abaixo a variável de ambiente é priorizada, então caso ela exista o caminho da string em output_json_env não importa:

  json_filename <- output_json_env %||% json_outfile

Quem usar função de fora e se esquecer que tem a variável de ambiente criada poderá ficar sem entender por que o arquivo json está sendo criado em uma pasta diferente da que ele está especificando no parâmetro.

E inverter a expressão para priorizar json_outfile ignora a existência da variável de ambiente.

Eu poderia ter usado outra lógica de implementação ou usar parâmetros separadas para escrita e caminho, etc. Mas tentei a forma acima até por parecer mais elegante usar o operador %||%.

Essa lógica da variável de ambiente sobrescrever o valor do parâmetro json_outfile é esperada? Creio que posso ter me confundido durante os assuntos da reunião quanto a ter somente um parâmetro para o json ou a forma de usá-lo.

[labanca]

@labanca porque vc achou importante criar um novo issue no lugar de usar o #49?

Verdade, criei esse issue pensando somente na nossa reunião e não me atentei a isso.

[labanca]

Ficou decidido que o a função check_results irá priorizar a existência da variável de ambiente LOG_FILE sobre o parâmetro json_outfile:

check_result <- function(df, report, status = "ok", stop_on_failure, output, summary = NULL,
                         json_outfile = NULL, log_level = "ERROR", 
                         msg_template = "Foram encontrados erros no teste.") {

  check_summary <- validate::summary(report)

  [...]

  output_json_env = ifelse(Sys.getenv("LOG_FILE") == "", NULL, Sys.getenv("LOG_FILE"))

  # environment variable overlaps the param to write the json
  json_outfile <- json_outfile %||% output_json_env

  # Write failure messages as a JSON Lines file 
  if (!is.null(json_outfile) && !valid) {

    con <- file(json_filename, open = "a", encoding = "UTF-8")  
    for (i in seq_len(nrow(fail))) {
      log_entry <- list(
        type = as.character(sys.calls()[[sys.parent()]][[1]]),
        log_level = log_level,
        timestamp = Sys.time(),
        message = glue_data(fail[i, ], msg_template),
        valid = valid
      )
      writeLines(jsonlite::toJSON(log_entry, auto_unbox = TRUE), con)
    }
    close(con)
  }

  result
}

[fjuniorr]

@labanca cria um PR fechando esse issue com o conteúdo de https://github.com/splor-mg/checks-planejamento/tree/json-log.

Fechei https://github.com/splor-mg/checks-planejamento/issues/49 e https://github.com/splor-mg/atividades/issues/135 que tem discussões relacionadas mas que ficaram em issues separados.

[labanca]

As funções de teste foram adaptadas para abarcar o novo output em json lines e mensagens de erro customizáveis. Exemplificando o caso geral com a função check_indicadores_previsoes_zeradas:

#' Verifica se indicadores não possuem previsões zeradas
#'
#' @export
check_indicadores_previsoes_zeradas <- function(indicadores_planejamento, output = FALSE, stop_on_failure = FALSE,
                                                json_outfile = NULL, log_level = "ERROR",
                                                msg_template = NULL) {
  df <- indicadores_planejamento |>
    filter(is_deleted_programa == FALSE &
                    is_deleted_indicador == FALSE)

  report <- check_that(df, 
                                 if(is_em_apuracao_ano0 == FALSE) previsao_para_ano0 != 0,
                                 if(is_em_apuracao_ano1 == FALSE) previsao_para_ano1 != 0,
                                 if(is_em_apuracao_ano2 == FALSE) previsao_para_ano2 != 0,
                                 if(is_em_apuracao_ano3 == FALSE) previsao_para_ano3 != 0)

  default_message = "Foram encontrados erros no teste."

  # prioritize the parameter error message, if used
  msg_template = msg_template %||% default_message

  check_result(df, report, stop_on_failure = stop_on_failure, output = output,
               json_outfile = json_outfile, log_level = log_level, msg_template = msg_template)
}

A mensagem de erro presente no corpo da função (default_message) será utilizada caso não seja passado um outro template de mensagem como parâmetro para a função de teste.

[fjuniorr]

Não consigo visualizar um caso em que o código que está chamando a função check_indicadores_previsoes_zeradas estaria interessado em passar um valor para msg_template porque isso exige informação sobre os detalhes de implementação da check_indicadores_previsoes_zeradas (eg. quais variáveis estão disponíveis).

Isso foi pra facilitar a implementação de alguma forma?

[labanca]

Não consigo visualizar um caso em que o código que está chamando a função check_indicadores_previsoes_zeradas estaria interessado em passar um valor para msg_template porque isso exige informação sobre os detalhes de implementação da check_indicadores_previsoes_zeradas (eg. quais variáveis estão disponíveis).

Isso foi pra facilitar a implementação de alguma forma?

Foi para dar flexibilidade de uso, já que quem usará o teste em geral será da equipe e terá conhecimento sobre ele (considerando um cenário que não haverá documentação da função também). Isso possibilita algum uso esporádico em que a mensagem de erro pode ser customizada sem precisar alterar o código do pacote (de um ou de todos os testes).

Além do que msg_template é opcional e não muda usos anteriores que o teste possa ter tido, já que a assinatura permanece a mesma.

Durante a oficina inclusive as mensagens foram testadas utilizando esses parâmetro antes de alterá-lo de fato no código do pacote checks-planejamento.