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

feat: IAM-286 preliminary commit for charm base #15

Merged
merged 4 commits into from
Oct 12, 2023
Merged

Conversation

wood-push-melon
Copy link
Contributor

@wood-push-melon wood-push-melon commented Sep 30, 2023

What

This pull request aims to provide the preliminary version of the GLAuth base charm.

I'm sorry this pull request is chunky since it's based on the code used in the demo. Please focus on the code under src when reviewing.

What's inside

  • Basic juju event handling
  • Postgresql charm relation
  • Kubernetes ConfigMap for GLAuth configuration
  • Integration with COS (sorry, this is because the code is based on the demo)

Improvements on the way

@wood-push-melon wood-push-melon requested a review from a team September 30, 2023 22:09
@wood-push-melon wood-push-melon changed the title feat: IAM-286 initial commits for charm base feat: IAM-286 preliminary commit for charm base Oct 1, 2023
Copy link
Contributor

@nsklikas nsklikas left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, but I have some comments. I suppose we are going to rework the kubernetes_resource logic either way (we should use the same logic in all of our charms).

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
src/validators.py Outdated Show resolved Hide resolved
src/validators.py Outdated Show resolved Hide resolved
src/validators.py Outdated Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
src/kubernetes_resource.py Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/validators.py Outdated Show resolved Hide resolved
src/kubernetes_resource.py Outdated Show resolved Hide resolved
src/validators.py Outdated Show resolved Hide resolved
Copy link
Contributor

@nsklikas nsklikas left a comment

Choose a reason for hiding this comment

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

One final comment. Also a lot of functions are missing return type annotations (and there are some other typing errors). Running mypy should show you whatever issues there are. It does not make sense to fix some of them, but let's try to resolve as many as possible.

config.yaml Outdated Show resolved Hide resolved
@wood-push-melon wood-push-melon merged commit 6664814 into main Oct 12, 2023
3 checks passed
@wood-push-melon wood-push-melon deleted the IAM-489 branch October 12, 2023 23:33
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