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

Resolve linter issues (ansible-lint, yamllint, flake8) #315

Merged
merged 54 commits into from
May 24, 2023
Merged

Resolve linter issues (ansible-lint, yamllint, flake8) #315

merged 54 commits into from
May 24, 2023

Conversation

ThomasSanson
Copy link
Sponsor Contributor

Close #308

The Dockerfile installs Docker and other packages required for development. It also creates a gitpod user with UID 33333.
🔧 chore(gitpod.yml): update Dockerfile path to match new location
The .gitpod.yml file is updated to reflect the new location of the Dockerfile.
…cessary hyphens and quotes

The changes in these playbooks are purely cosmetic, removing unnecessary hyphens and quotes to improve readability and consistency. This makes the playbooks easier to read and understand, which can help with maintenance and future development.
…Ansible linting warnings

The `run_once` tasks in the playbook were updated to include a `noqa run-once` comment to suppress Ansible linting warnings. This improves the readability of the playbook and reduces the noise generated by the linter.
The playbook now has a descriptive name "Add load balancer" to improve readability and understanding of the playbook's purpose. The host section now has a name "new_balancer" to improve the clarity of the playbook's target.
…nd traceability

The first task name has been updated to include the name of the playbook file to improve traceability. The second task name has been updated to include more descriptive language to improve readability.
…cation warning

The include keyword is deprecated and will be removed in future versions of Ansible. The include_tasks keyword should be used instead to include tasks from another file.
…s to tasks

The changes in this commit improve the readability of the playbook by adding descriptive names to the tasks. The name of the task now reflects the purpose of the task, making it easier to understand what is happening in the playbook. Additionally, the changes make the playbook more modular by importing the Consul playbook instead of duplicating the code.
…aceability

The commit adds task names to the playbook to improve readability and traceability of the tasks. The task names are now prefixed with the name of the playbook, which makes it easier to identify which playbook a task belongs to.
@vitabaks vitabaks changed the title Linters Resolve linter issues (ansible-lint, yamllint, flake8) May 4, 2023
ThomasSanson and others added 21 commits May 5, 2023 18:38
The line "---" was removed from the beginning of the file as it was unnecessary.
…improve readability

The name key was added to the playbook to provide a clear and concise description of what the playbook does. The hosts key was updated to use a more descriptive name to improve readability and make it easier to understand what the playbook is targeting.
…ve readability

The playbook name in balancers.yml has been updated to include the filename for better readability.
The changes in this commit remove unnecessary whitespace and a line that was not needed in the configuration file. This improves the readability and maintainability of the code.
…ove task names

The playbook name now includes the file name to improve clarity and the task names have been improved to better describe what they do.
The playbook name was misspelled as "balancer.yml" instead of "balancers.yml". This commit fixes the typo to improve the readability and clarity of the playbook name.
… descriptive names to tasks

The playbook has been updated to include descriptive names for each task. This improves the readability of the playbook and makes it easier to understand what each task is doing. The names have been added to the tasks that configure the Postgres cluster, deploy the Postgres cluster, and configure HAProxy load balancing.
The include keyword is deprecated and should be replaced with include_tasks. This commit updates the handlers in the config_pgcluster.yml file to use include_tasks instead of include.
The changes in this commit are purely cosmetic. The commit removes unnecessary whitespace and comments in the consul.yml file to improve readability and maintainability.
…eadability

The name "Consul Playbook" was added to the playbook to improve readability and make it easier to identify the purpose of the playbook. The host names were also updated to be more descriptive and improve readability.
…eadability

The playbook name and task names have been updated to include the filename and to be more descriptive. This improves the readability of the playbook and makes it easier to understand what each task is doing.
The changes in this commit remove unnecessary lines in the deploy_pgcluster.yml file. Specifically, the removed lines are the name of the playbook and the ellipsis at the end of the file.
… better readability

The import_playbook tasks in the deploy_pgcluster.yml file have been given names to improve the readability of the playbook. The names now reflect the purpose of the tasks, making it easier to understand what each task does.
…readability

The playbook now has task names for each section to improve readability and make it easier to understand what each section does. The task names are "Postgres Cluster Configuration", "Install and configure pgBackRest", and "PostgreSQL Cluster Deployment".
…ter readability

The playbook name was added to all tasks in the deploy_pgcluster.yml file to improve readability and make it easier to identify which playbook a task belongs to.
…n empty list

The firewall_ports_dynamic_var variable is now set to a default value of an empty list to avoid errors when the variable is not defined. This change was made in multiple ansible files.
…n empty list

✨ feat(firewall): ensure firewall_additional_rules is unique
The firewall_additional_rules variable is now set to a default value of an empty list to avoid errors when the variable is not defined. The unique filter is also applied to the variable to ensure that there are no duplicate rules.
The playbook name has been updated to better reflect its purpose. The name change also improves readability and makes it easier to understand what the playbook does. A description has been added to provide more context and clarity.
…otes

The min_ansible_version and platform versions are now wrapped in quotes to ensure that they are interpreted as strings. This is important because some versions may contain characters that could be interpreted as numbers or other data types. Wrapping them in quotes ensures that they are interpreted as strings and avoids any potential issues.
The high-availability tag is being removed from the galaxy_tags list as it is not relevant to this role.
The exception message was formatted in a way that made it difficult to read. This commit fixes the formatting of the exception message to make it more readable.
The line break in the YeditException message was unnecessary and made the message harder to read. This commit removes the line break to improve readability.
The line break in the if statement was unnecessary and made the code harder to read. This commit removes the line break to improve readability.
…ability

The parentheses around the condition in the elif statement are unnecessary and have been removed to improve code readability.
…tement

The if statement was checking if an index was greater than the length of a list minus one. The subtraction operator was not surrounded by whitespace, which is not consistent with PEP8 style guide. This commit fixes the whitespace around the subtraction operator to improve code readability and consistency with PEP8.
The changes remove unnecessary line breaks in if statements to improve code readability.
The E402 error is now ignored in the flake8 configuration file. This change was made to avoid false positives in the codebase.
@ThomasSanson ThomasSanson marked this pull request as ready for review May 8, 2023 10:28
@ThomasSanson
Copy link
Sponsor Contributor Author

make lint ok

@vitabaks
Copy link
Owner

vitabaks commented May 8, 2023

@ThomasSanson consider offering a PR for yedit.py on the project repository

@ThomasSanson
Copy link
Sponsor Contributor Author

Hi @vitabaks, thank you for your comment.

I'm not quite sure if I understood your request correctly.

Are you suggesting that I should I cancel the changes I made in the current Pull Request ?

Please let me know so I can proceed accordingly. Thanks !

@vitabaks
Copy link
Owner

vitabaks commented May 8, 2023

not necessarily cancel these changes.

It would just be nice to suggest improvements to the original repository of this module as well.

@ThomasSanson
Copy link
Sponsor Contributor Author

I don't understand the connection between this repository and ansible-role-yedit, except maybe that you were inspired by it ?

In any case, I need to take a closer look at how everything is organized and study it because submitting a pull request to ansible-role-yedit doesn't seem simple, as it doesn't appear to be welcoming to contributions at the moment.
So, there's a lot of work to do.

Why didn't you include the role in a requirements.yml ?

I will create an issue on your repository so as not to forget

@vitabaks
Copy link
Owner

vitabaks commented May 8, 2023

in fact, this is not a role, but a library for managing the yaml configuration, which you can observe here https://github.com/vitabaks/postgresql_cluster/tree/master/roles/patroni/library

I don't think it should be defined in requirements.yml

ThomasSanson and others added 5 commits May 23, 2023 19:28
…abling TimescaleDB

✨ feat(config_pgcluster.yml): add support for TimescaleDB by adding 'timescaledb' to 'shared_preload_libraries' PostgreSQL parameter
The check for the PostgreSQL version is added to ensure that the current version is supported by TimescaleDB. If the version is not supported, the playbook will fail. The support for TimescaleDB is added by adding 'timescaledb' to the 'shared_preload_libraries' PostgreSQL parameter. This is done by modifying the 'postgresql_parameters' variable. If 'timescaledb' is not already present in the value of 'shared_preload_libraries', it is appended to the end of the value. If it is already present, the value is left unchanged.
The .vscode/settings.json file is now being ignored by git. This file is used to store user settings for the Visual Studio Code editor.
…abling TimescaleDB

✨ feat(deploy_pgcluster.yml): add support for TimescaleDB installation via repository
The playbook now checks the PostgreSQL version before enabling TimescaleDB to ensure that it is compatible. If the version is not compatible, the playbook will fail. Additionally, support for TimescaleDB installation via repository has been added. The playbook now ensures that 'timescaledb' is in 'shared_preload_libraries' when TimescaleDB is enabled.
…luster deployment test

The enable_timescale variable is set to true to enable testing of TimescaleDB cluster deployment. The variable is only set if the ansible_distribution is not Ubuntu 20.04, as there is a known issue with TimescaleDB on that version.
@ThomasSanson
Copy link
Sponsor Contributor Author

@vitabaks, I'm back from vacation ^^

I have just made the corrections for the merge conflicts, and everything is good to go for me to merge this pull request.

@vitabaks
Copy link
Owner

vitabaks commented May 23, 2023

@ThomasSanson Welcome back! I'm glad you're here)

please don't forget to remove the skip_list rules from .ansible-lint based on what has been fixed.

To have a clear understanding of what else needs attention.

@vitabaks vitabaks added the good first issue Good for newcomers label May 23, 2023
@ThomasSanson
Copy link
Sponsor Contributor Author

Hi @vitabaks,

Thank you for your comment.

I understand your concern regarding the skip_list rules in .ansible-lint. For now, I have just made the linters pass for this pull request.

My plan is to create a separate issue for each rule in skip_list after this pull request. This will allow us to address each issue systematically and have a clear understanding of what else needs attention.

I hope this addresses your concerns. Thank you again for your feedback.

config_pgcluster.yml Outdated Show resolved Hide resolved
deploy_pgcluster.yml Outdated Show resolved Hide resolved
ThomasSanson and others added 2 commits May 24, 2023 16:17
Co-authored-by: Vitaliy Kukharik <37010174+vitabaks@users.noreply.github.com>
Co-authored-by: Vitaliy Kukharik <37010174+vitabaks@users.noreply.github.com>
@vitabaks vitabaks merged commit 999b09a into vitabaks:master May 24, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolve all linter issues (ansible-lint, yamllint, flake8)
2 participants