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.

Sign up for GitHub

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

Jump to bottom

Improve func interface to have Arguments method and add other utils #1251

Merged
merged 16 commits into from
Mar 9, 2022
Merged

Improve func interface to have Arguments method and add other utils #1251

merged 16 commits into from
Mar 9, 2022

Conversation

viveksinghggits
Copy link
Contributor

@viveksinghggits viveksinghggits commented Feb 16, 2022 •
edited

Change Overview

This PR refactors the func interface in kanister to add another method named Arguments() []string that is going to get us all the arguments that are supported by a kanister function.
It also adds a simple function that lists all the kanister functions that registered and can be used in blueprints.

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

  • #XXX

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@viveksinghggits
Copy link
Contributor Author

Hi @pavannd1,
Like RequiredArgs() []string method for Kanister function type (type Func interface) I propose to have another method just Arguments() []string that is going to return all the arguments that are supported for a kanister functions.
This would be really helpful to validate blueprints

@viveksinghggits viveksinghggits changed the title Add function to list all the registered kanister functions Improve func interface to have Arguments method and add other utils Feb 21, 2022
pavannd1
pavannd1 reviewed Feb 22, 2022
View reviewed changes
pkg/function/backup_data.go Outdated Show resolved Hide resolved
pkg/function/backup_data_all.go Outdated Show resolved Hide resolved
pkg/function/backup_data_stats.go Outdated Show resolved Hide resolved
pkg/function/copy_volume_data.go Outdated Show resolved Hide resolved
pkg/function/create_csi_snapshot.go Outdated Show resolved Hide resolved
pkg/function/prepare_data.go Outdated Show resolved Hide resolved
pkg/function/restore_data.go Outdated Show resolved Hide resolved
pkg/function/restore_data_all.go Outdated Show resolved Hide resolved
pkg/function/restore_rds_snapshot.go Outdated Show resolved Hide resolved
pkg/kanister.go Show resolved Hide resolved
pavannd1
pavannd1 reviewed Feb 23, 2022
View reviewed changes
build/integration-test.sh Outdated Show resolved Hide resolved
pkg/phase.go Outdated Show resolved Hide resolved
pkg/phase.go Outdated Show resolved Hide resolved
pkg/phase.go Outdated Show resolved Hide resolved
pkg/kanister.go Show resolved Hide resolved
@viveksinghggits
Address review comment, rename function
cafc200
@viveksinghggits
Fix controller test that failed becaue of unsupported arg
d2c295d 
The mock function that we were using in the controller test didn't
return any argument, and because of that the execution of actionset
failed.
This fixes that.
@viveksinghggits
Add utility method to get Func type from fun name
805c2be
ihcsim
ihcsim reviewed Feb 24, 2022
View reviewed changes
Copy link
Contributor

@ihcsim ihcsim left a comment

Choose a reason for hiding this comment

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

Minor feedback. Otherwise, LGTM.

pkg/kanister.go Outdated Show resolved Hide resolved
pkg/kanister.go Show resolved Hide resolved
pavannd1
pavannd1 reviewed Mar 8, 2022
View reviewed changes
Copy link
Contributor

@pavannd1 pavannd1 left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

@ihcsim ihcsim added the kueue label Mar 8, 2022
@mergify mergify bot merged commit 67ecd2a into master Mar 9, 2022
@mergify mergify bot deleted the funname-list branch March 9, 2022 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ihcsim ihcsim ihcsim approved these changes

@pavannd1 pavannd1 pavannd1 approved these changes

@ankitjain235 ankitjain235 Awaiting requested review from ankitjain235

Assignees
No one assigned
Labels
kueue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@viveksinghggits @ihcsim @pavannd1