Skip to content

added optional session_info to create_unknow_user #135

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

Closed

Conversation

yvess
Copy link

@yvess yvess commented Oct 18, 2018

Calls create_unknow_user() with option session_info if the create_unknow_user function has one argument. This allows for a lot of flexibility for the create_unknow_user function to write code that depends on data from session_info. The docs are also updated.

@knaperek
Copy link
Collaborator

Thanks for your effort @yvess.

I understand that having more context could be helpful in your setup and at the same time it doesn't feel right to me to pull out guns like inspect to achieve this; not in a reference library like this.

Rather than handling all special cases that anyone can have in more complex systems right here in the default ACS view and then possibly breaking the code with more updates, I suggest everybody writes their own ACS view (taking this one as an inspiration only). I found that to be the most straightforward approach after all. It's really not that hard and if you are serious about your project, you will probably have many specific ideas and differencies you will like to implement and improve later on.
I've been there :-)

@peppelinux
Copy link
Member

Hi @yvess, can you refactor this code without the use of inspect?
Probably this will incourage @knaperek and letting us to merge your contribution. There would also the need of unit tests if possibile.

thank you

@peppelinux peppelinux added this to the v0.18.2 milestone Apr 29, 2020
@peppelinux peppelinux force-pushed the master branch 6 times, most recently from 0dcbdc6 to 02515a2 Compare May 1, 2020 16:25
@peppelinux peppelinux force-pushed the master branch 3 times, most recently from 4c6e98f to 31e9254 Compare July 30, 2020 23:34
@peppelinux
Copy link
Member

Too much time without a reply, Feel free to re open this if needed. Thank you for the time you spent on this project despite everything

@peppelinux peppelinux closed this Oct 20, 2020
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.

3 participants