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

testing: lints partial fixes #22

Merged
merged 1 commit into from
Aug 29, 2023
Merged

Conversation

ccamacho
Copy link
Collaborator

This commit resolves the following lint rules:

  • 'var-naming[no-reserved]'
  • 'var-naming[no-role-prefix]'
  • 'var-naming[pattern]'
  • 'yaml[colons]'
  • 'yaml[commas]'
  • 'yaml[comments]'
  • 'yaml[empty-lines]'
  • 'yaml[indentation]'
  • 'yaml[key-duplicates]'
  • 'yaml[line-length]'
  • 'yaml[octal-values]'
  • 'name[template]'
  • 'name[casing]'

Partially-solves: #10

@openshift-ci
Copy link

openshift-ci bot commented Aug 29, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign fcami for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci
Copy link

openshift-ci bot commented Aug 29, 2023

Hi @ccamacho. Thanks for your PR.

I'm waiting for a openshift-psap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 29, 2023
@kpouget
Copy link
Contributor

kpouget commented Aug 29, 2023

thanks @ccamacho , looks all good :)
I only had one question ... but I found the answer 🙃

* name[template] - Jinja templates should only be at the end of 'name'. This helps with the identification of tasks inside the source code when they fail. The use of templating inside name keys is discouraged as there are multiple cases where the rendering of the name template is not possible.

I'll launch a test run just to be on the safe side

@kpouget
Copy link
Contributor

kpouget commented Aug 29, 2023

/test watsonx-serving-light

@kpouget
Copy link
Contributor

kpouget commented Aug 29, 2023

@ccamacho, not a flake this time:

2023-08-29 07:38:43,813 p=271 u=psap-ci-runner n=ansible | 
2023-08-29 07:38:43,813 p=271 u=psap-ci-runner n=ansible | ---
2023-08-29 07:38:43,813 p=271 u=psap-ci-runner n=ansible | roles/cluster/cluster_set_scale/tasks/main.yml:74
2023-08-29 07:38:43,813 p=271 u=psap-ci-runner n=ansible | TASK: cluster_set_scale : Get first machineset
2023-08-29 07:38:43,813 p=271 u=psap-ci-runner n=ansible | ----- FAILED ----
2023-08-29 07:38:43,814 p=271 u=psap-ci-runner n=ansible | msg: Unexpected templating type error occurred on (Change all {{ machineset_instance_type + 'machinesets replicas to have sum' + scale }}): can only concatenate str (not "int") to str
2023-08-29 07:38:43,814 p=271 u=psap-ci-runner n=ansible | ----- FAILED ----

@ccamacho
Copy link
Collaborator Author

ccamacho commented Aug 29, 2023

From the CI job the cast was failing with:

---
roles/cluster/cluster_set_scale/tasks/main.yml:74
TASK: cluster_set_scale : Get first machineset
----- FAILED ----
msg: Unexpected templating type error occurred on (Change all {{ machineset_instance_type + 'machinesets replicas to have sum' + scale }}): can only concatenate str (not "int") to str
----- FAILED ----
PLAY RECAP *********************************************************************
localhost                  : ok=18   changed=13   unreachable=0    failed=1    skipped=9    rescued=0    ignored=0   
---

@ccamacho
Copy link
Collaborator Author

@ccamacho, not a flake this time:

2023-08-29 07:38:43,813 p=271 u=psap-ci-runner n=ansible | 
2023-08-29 07:38:43,813 p=271 u=psap-ci-runner n=ansible | ---
2023-08-29 07:38:43,813 p=271 u=psap-ci-runner n=ansible | roles/cluster/cluster_set_scale/tasks/main.yml:74
2023-08-29 07:38:43,813 p=271 u=psap-ci-runner n=ansible | TASK: cluster_set_scale : Get first machineset
2023-08-29 07:38:43,813 p=271 u=psap-ci-runner n=ansible | ----- FAILED ----
2023-08-29 07:38:43,814 p=271 u=psap-ci-runner n=ansible | msg: Unexpected templating type error occurred on (Change all {{ machineset_instance_type + 'machinesets replicas to have sum' + scale }}): can only concatenate str (not "int") to str
2023-08-29 07:38:43,814 p=271 u=psap-ci-runner n=ansible | ----- FAILED ----

xD Sorry didnt see this msg :)

@kpouget
Copy link
Contributor

kpouget commented Aug 29, 2023

/test watsonx-serving-light

@kpouget
Copy link
Contributor

kpouget commented Aug 29, 2023

something wrong happened with the (my) code parsing the GH comments :/ I'll have to turn this into Python sooner or later

@kpouget
Copy link
Contributor

kpouget commented Aug 29, 2023

/test watsonx-serving-light

@kpouget
Copy link
Contributor

kpouget commented Aug 29, 2023

oh, I get it now, I thought the script was catching my comment not a flake this time, but it's catching yours, as expected 😅 . Well, not really as expected, because it should search for the /test watsonx-serving-light prefix 🤔

Can you launch the test, without anything else in the comment?

@ccamacho
Copy link
Collaborator Author

/test watsonx-serving-light

@openshift-ci
Copy link

openshift-ci bot commented Aug 29, 2023

@ccamacho: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test watsonx-serving-light

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kpouget
Copy link
Contributor

kpouget commented Aug 29, 2023

/test watsonx-serving-light

@ccamacho
Copy link
Collaborator Author

The ci/prow/watsonx-serving-light job Passed I will rebase this as it is ood.

This commit resolves the following lint rules:

- 'var-naming[no-reserved]'
- 'var-naming[no-role-prefix]'
- 'var-naming[pattern]'
- 'yaml[colons]'
- 'yaml[commas]'
- 'yaml[comments]'
- 'yaml[empty-lines]'
- 'yaml[indentation]'
- 'yaml[key-duplicates]'
- 'yaml[line-length]'
- 'yaml[octal-values]'
- 'name[template]'
- 'name[casing]'

Partially-solves: openshift-psap#10
@kpouget
Copy link
Contributor

kpouget commented Aug 29, 2023

thanks for your help @ccamacho, merging the PR :)

@kpouget kpouget merged commit 666961b into openshift-psap:main Aug 29, 2023
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants