Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix yamlfix and create_yaml_file #545

Merged
merged 7 commits into from
Jun 13, 2024
Merged

Fix yamlfix and create_yaml_file #545

merged 7 commits into from
Jun 13, 2024

Conversation

aspeddro
Copy link
Contributor

@aspeddro aspeddro commented May 27, 2024

Esse PR tem quatro mudanças:

  • Adiciona um script que a Laura tinha feito para subir as colunas para Django
  • Mudança em create_yaml_file.py
    • Aumenta a indentação e setando yaml_obj.explicit_start = True
    • O motivo é que ao executar create_yaml_file.py ele altera o formato do dbt_project.yml
  • Remove gists/requirements-gists.txt. Utilizamos o poetry como gerenciador de pacotes, então esse arquivo não é mais necessário.
    • As dependências agora estão em pyproject.yml
  • A verificação do yamlfix não está correta. Ele estava formatando os arquivos quando deveria fazer uma verificação, i.e, --check flag.

@aspeddro aspeddro self-assigned this May 27, 2024
Copy link
Contributor

@laura-l-amaral laura-l-amaral left a comment

Choose a reason for hiding this comment

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

  • Pq a verificação do yamlfix deveria ser um check e não uma correção? Me parece mais prático que ele já falça a correção direta
  • Outra questão é que esse código que vc adicionou já está no repo de pipelines

@@ -0,0 +1,151 @@
import basedosdados as bd
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ops, não tinha visto. Não é melhor deixar ele nesse repositório? Já tem vários scripts em gists/

Copy link
Contributor

@laura-l-amaral laura-l-amaral Jun 5, 2024

Choose a reason for hiding this comment

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

então, a questão é que o script usa algumas funções que estão no repositório de pipelines, por isso deixei por lá. Vc ve alguma maneira de solucionar isso pra deixar o script nesse repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Vi o script e tem duas funções lá que já estão aqui:

  • read_architecture_table, já tem aqui
  • get_headers, também já tá aqui

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm acho que o get headers só funciona se tiver o .env que está no repo de pipelines ativado, isso nao seria um problema?

Copy link
Contributor

Choose a reason for hiding this comment

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

Onde está a função de get_headers?

Copy link
Contributor Author

@aspeddro aspeddro Jun 12, 2024

Choose a reason for hiding this comment

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

Engano meu. Em pipelines é get_headers, aqui é get_token

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Só nome diferente

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aqui a pessoa vai rodar o script python gists/upload_columns_django.py e ele vai pedi o password no prompt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm acho que o get headers só funciona se tiver o .env que está no repo de pipelines ativado, isso nao seria um problema?

Em pipelines sim, mas aqui ele vai pedi o password do django da pessoa

@aspeddro
Copy link
Contributor Author

aspeddro commented Jun 5, 2024

Pq a verificação do yamlfix deveria ser um check e não uma correção?

Pq a correção é feita pelo pre-commit. Todas vez que você faz uma alteração e faz um commit o pre-commit é executado localmente, só que é possível fazer um commit sem executar o pre-commit, usando a flag -n

-n, --[no-]verify
           By default, the pre-commit and commit-msg hooks are run. When any of --no-verify or -n is given, these are bypassed. See also githooks(5)

Então para não mesclar arquivos não formatados, fazermos um verificação no GitHub Action. Isso é feito com arquivos .sql

- name: Lint sql
run: poetry run sqlfmt --diff .

Ele verifica se o sql foi formatado. Para o yamlfix ele formata os arquivos na action, aqui um exemplo. Se fosse uma verificação não teria passado. Indicando para pessoa que ela deve formatar o arquivo.

@laura-l-amaral
Copy link
Contributor

Pq a verificação do yamlfix deveria ser um check e não uma correção?

Pq a correção é feita pelo pre-commit. Todas vez que você faz uma alteração e faz um commit o pre-commit é executado localmente, só que é possível fazer um commit sem executar o pre-commit, usando a flag -n

-n, --[no-]verify
           By default, the pre-commit and commit-msg hooks are run. When any of --no-verify or -n is given, these are bypassed. See also githooks(5)

Então para não mesclar arquivos não formatados, fazermos um verificação no GitHub Action. Isso é feito com arquivos .sql

- name: Lint sql
run: poetry run sqlfmt --diff .

Ele verifica se o sql foi formatado. Para o yamlfix ele formata os arquivos na action, aqui um exemplo. Se fosse uma verificação não teria passado. Indicando para pessoa que ela deve formatar o arquivo.

acho que saquei, só mais uma dúvida, como que ele formata os arquivos na action e o código continua fora do padrão?

@aspeddro
Copy link
Contributor Author

Pq ao formatar a gente não adicionou mais passos depois disso.

A última linha é essa:

run: poetry run yamlfix --exclude ".kubernetes/**/*" .

Depois de formatar podemos commitar os arquivos alterados na action e enviar para algum lugar, em geral no próprio PR, mas isso não foi feito (concordo). Funciona como um repo git padrão.

@aspeddro aspeddro changed the title Add script upload_columns_django.py Fix yamlfilx and create_yaml_file Jun 13, 2024
@aspeddro aspeddro changed the title Fix yamlfilx and create_yaml_file Fix yamlfix and create_yaml_file Jun 13, 2024
@aspeddro aspeddro merged commit 8c65a1b into main Jun 13, 2024
2 checks passed
tricktx pushed a commit that referenced this pull request Jul 12, 2024
* increase offset

* add upload_columns_django.py

* remove requirements-gists.txt

* yamlfix: add --check flag and format

* rm upload_columns_django.py

* update
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.

2 participants