Skip to content

Helpers and keywords unification#75

Merged
m-wcislo merged 29 commits into
masterfrom
helpers-cleanup
Nov 30, 2021
Merged

Helpers and keywords unification#75
m-wcislo merged 29 commits into
masterfrom
helpers-cleanup

Conversation

@m-wcislo

@m-wcislo m-wcislo commented Apr 17, 2021

Copy link
Copy Markdown
Collaborator

@Nilsty, @satish-nubolab, @mika-b - I'm planning to cleanup the keywords and helpers a little bit to make whole thing more DRY, clean etc. The idea is to make all the keywords always return k8s objects not their names in some cases. To get names or anything more specific we should use helpers (filters).

The old functions will be marked deprecated and kept till v1.0.0. Let me know your thoughts (also keyword naming convention proposal are welcome). Currently the PR has minimal POC version to show how it would look like and what are the efforts.

Before merge following needs to be applied:

  • At least one example testcase added in testcases/
  • Library Documentation regenerated according to Generate docs
  • All new testcases tagged as prerelease along other tags to exclude it from execution until released on PyPI
  • Coverage threshold increased in .coveragerc if new coverage is higher than actual, see the lint-and-coverage step in CI
fail_under = 67

@Nilsty

Nilsty commented Aug 14, 2021

Copy link
Copy Markdown
Collaborator

I like the idea of always returning the full k8s object or list of objects for the mail keywords. When using the KubeLibrary I personally don't use any of the filter keywords and rather filter the k8s object myself in custom keywords. From my point of view the filter keywords are not needed. Having good test cases examples to build custom filters works better for me.

My naming proposal would be to keep it close to the kubernetes api function names. e.g. Get Services In Namespace should be List Namespaced Services and Get Service Details In Namespace should be Read Namespaced Service.

@m-wcislo
m-wcislo requested a review from Nilsty November 18, 2021 10:19
@Nilsty

Nilsty commented Nov 22, 2021

Copy link
Copy Markdown
Collaborator

@m-wcislo The changes look good. Good job. I really like the new filter keywords e.g. filter_names and filter_by_key.

@m-wcislo

Copy link
Copy Markdown
Collaborator Author

@Nilsty docs generted and release prepared, please approve.

@Nilsty Nilsty left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@m-wcislo
m-wcislo merged commit f97084a into master Nov 30, 2021
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.

2 participants