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

Refactor arangodb scaler config #5808

Merged
merged 2 commits into from
May 29, 2024

Conversation

dttung2905
Copy link
Contributor

@dttung2905 dttung2905 commented May 15, 2024

Refactor arango scaler config reading with a better way

Checklist

Relates to #5797

Copy link
Member

@wozniakjan wozniakjan left a comment

Choose a reason for hiding this comment

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

nice, looking rather good! :)

pkg/scalers/arangodb_scaler.go Outdated Show resolved Hide resolved
pkg/scalers/arangodb_scaler.go Outdated Show resolved Hide resolved
pkg/scalers/arangodb_scaler.go Show resolved Hide resolved
@dttung2905
Copy link
Contributor Author

dttung2905 commented May 21, 2024

/run-e2e arangodb
Update: You can check the progress here

@dttung2905 dttung2905 changed the title WIP: Refactor arangodb scaler config Refactor arangodb scaler config May 21, 2024
Copy link
Member

@wozniakjan wozniakjan left a comment

Choose a reason for hiding this comment

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

lgtm, well done! :)

I noticed few nits, to make the review easier, I submitted them as a PR to your PR - ptal dttung2905#1

pkg/scalers/arangodb_scaler.go Show resolved Hide resolved
pkg/scalers/arangodb_scaler.go Show resolved Hide resolved
@wozniakjan
Copy link
Member

ptal @kedacore/keda-contributors, I think this is good to merge

@zroubalik
Copy link
Member

zroubalik commented May 27, 2024

/run-e2e arangodb
Update: You can check the progress here

@zroubalik zroubalik enabled auto-merge (squash) May 27, 2024 10:28
auto-merge was automatically disabled May 27, 2024 20:42

Head branch was pushed to by a user without write access

@dttung2905
Copy link
Contributor Author

dttung2905 commented May 27, 2024

/run-e2e arangodb
Update: You can check the progress here

@dttung2905
Copy link
Contributor Author

Not too sure why this e2e fails all of the sudden though https://github.com/kedacore/keda/actions/runs/9253331273/job/25452795586

0s
Run MESSAGE="$COMMENT_BODY"
  MESSAGE="$COMMENT_BODY"
  REGEX='/run-e2e (.+)'
  if [[ "$MESSAGE" =~ $REGEX ]]
  then
    export E2E_TEST_REGEX="$(echo ${BASH_REMATCH[[1](https://github.com/kedacore/keda/actions/runs/9253331273/job/25452795586#step:7:1)]} | head -1)"
  fi
  make e[2](https://github.com/kedacore/keda/actions/runs/9253331273/job/25452795586#step:7:2)e-regex-check
  shell: sh -e {0}
  env:
    E2E_CHECK_NAME: e2e tests
    COMMENT_BODY: /run-e2e arangodb
make: *** No rule to make target 'e2e-regex-check'.  Stop.
Error: Process completed with exit code 2.

The E2E_TEST_REGEX return arangodb from my local when I replace $COMMENT_BODY with /run-e2e arangodb which is expected 🤔
@JorTurFer Do you have any idea about this ? I can see the same thing is happening on other PR as well https://github.com/kedacore/keda/actions/runs/9259917601

@wozniakjan
Copy link
Member

looks like #5783 introduced validation on the regex passed to the e2e tests, which seems to be misbehaving now, I'm checking this

@wozniakjan
Copy link
Member

@dttung2905 there appears to be a combination of two things

  • fix: e2e test regex check tag #5831 - bad tag argument passed in the makefile
  • after that merges, can you please rebase your PR on the latest main? The workflow code executes from main but it runs actions/checkout to get the content of the PR. While the workflow already has make e2e-regex-check, this branch doesn't yet have that in the Makefile hence the failure
make: *** No rule to make target 'e2e-regex-check'.  Stop.

Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905
Copy link
Contributor Author

dttung2905 commented May 28, 2024

/run-e2e arangodb
Update: You can check the progress here

@dttung2905
Copy link
Contributor Author

ptal @kedacore/keda-contributors. Please help to merge when you have time 🙏

@zroubalik zroubalik merged commit 65c8caa into kedacore:main May 29, 2024
20 checks passed
@dttung2905 dttung2905 deleted the refactor-arangodb-scaler branch May 29, 2024 11:49
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.

None yet

4 participants