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

Allow to override nomad_artifact and nomad_ui #168

Closed
wants to merge 9 commits into from
Closed

Allow to override nomad_artifact and nomad_ui #168

wants to merge 9 commits into from

Conversation

AlbanAndrieu
Copy link
Contributor

@AlbanAndrieu AlbanAndrieu commented Aug 17, 2023

  • Allow to override nomad_artifact and nomad_ui, nomad 1.6.1 introduce https://developer.hashicorp.com/nomad/docs/configuration/ui, her it allow to change the color
  • Allow to ignore mounting on failure such as /var/run/docker.sock (already exists) with a boolean nomad_host_volumes_skip_ignore, default behavior will stay the same

@AlbanAndrieu
Copy link
Contributor Author

Hi @Rendanic, @lanefu, @ctorrisi how can I have a review?

@AlbanAndrieu
Copy link
Contributor Author

@CodiumAI-Agent /review [-i]: Request a review of your Pull Request.

@CodiumAI-Agent
Copy link

PR Analysis

  • 🎯 Main theme: This PR allows the override of nomad_artifact and nomad_ui, and introduces the option to ignore mounting failures.
  • 📌 Type of PR: Enhancement
  • Focused PR: Yes, all changes are related to the enhancement of Nomad configuration.
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • General suggestions: The PR introduces useful enhancements to the Nomad configuration. However, it would be beneficial to add tests to ensure the new features work as expected and do not introduce regressions. Additionally, consider adding comments in the code to explain the purpose of the new variables and conditions.

  • 🤖 Code feedback:

    • relevant file: defaults/main.yml
      suggestion: Consider adding default values for the new variables nomad_artifact and nomad_host_volumes_skip_ignore to ensure they have a value if not explicitly set. [medium]
      relevant line: nomad_host_volumes_skip_ignore: false

    • relevant file: handlers/main.yml
      suggestion: The ignore_errors option should not be set to ansible_check_mode. Instead, it should be a boolean value or a variable that can be set to true or false. [important]
      relevant line: ignore_errors: "{{ ansible_check_mode }}"

    • relevant file: templates/client.hcl.j2
      suggestion: It would be better to check if nomad_artifact is not empty before iterating over its items. This can prevent potential errors if nomad_artifact is not defined. [medium]
      relevant line: {% if nomad_artifact -%}

How to use

Tag me in a comment '@CodiumAI-Agent' and add one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest improvements to the code in the PR. Extended mode employs several calls, and provides a more thorough feedback.
/ask <QUESTION>: Pose a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.

To edit any configuration parameter from configuration.toml, add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, use the /config command.

@AlbanAndrieu
Copy link
Contributor Author

I resolved the conflict. @beechesII feel free to review this MR are redirect me to the right person.

@jusmundi jusmundi closed this by deleting the head repository Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants