-
Notifications
You must be signed in to change notification settings - Fork 0
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
PB-1044 Add interface for cognito #29
Conversation
1b36237
to
474a0e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice so far!! 🚀
I have a few remarks in the code.
Some thought I just had:
Would it be good to have an attribute in the pool that is set when the user is managed from service control? So that when we clear and remove users, we only take the ones that have this attribute/flag.
Like this it would be possible to have manually created users that won't be removed by the sync command. This could prove to be handy for debugging purposes, or when we need an admin user of some sort.
(FYI. the build currently fails because of the TODOs in the code)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
Added, as discussed, a |
47df145
to
91fce2e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
56abee6
to
1c68348
Compare
Added:
cognito.utils.Service
for managing cognito userscognito.utils.user
to be used in the API when creating, deleting and updating a usercognito_sync
for synchronizing users (same base classes used as in PB-1026 Add BOD import command #23, taken fromservice-stac
)cognito-local
for local testing and development