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

chore(commands): fix user name display in commands #2284

Merged
merged 13 commits into from
Jan 16, 2025

Conversation

maugde
Copy link
Contributor

@maugde maugde commented Jan 7, 2025

fix ANT-2543

@maugde maugde self-assigned this Jan 7, 2025
@maugde maugde requested a review from TheoPascoli January 7, 2025 15:35
Copy link
Member

@sylvlecl sylvlecl left a comment

Choose a reason for hiding this comment

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

I think we should have a slightly more generic approach so that all tasks fill the _current_user context var, see comments.
Later, this will ensure that all tasks have the current user available, not only TS generation tasks. This will also simplify the implementation of execute_or_add_commands

antarest/study/service.py Outdated Show resolved Hide resolved
antarest/study/business/utils.py Outdated Show resolved Hide resolved
@maugde maugde force-pushed the fix/ANT-2543-fix-user-name-display-in-commands branch from ba13afe to c37e3c0 Compare January 9, 2025 09:40
@pull-request-size pull-request-size bot added size/L and removed size/XL labels Jan 9, 2025
@maugde maugde requested a review from sylvlecl January 9, 2025 11:20
@maugde maugde force-pushed the fix/ANT-2543-fix-user-name-display-in-commands branch 2 times, most recently from e0fa0f6 to 63e4baa Compare January 13, 2025 08:31
Copy link
Member

@sylvlecl sylvlecl left a comment

Choose a reason for hiding this comment

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

The new implementation looks good!
Still some last comments, in particular I think we should add a TODO to clarify more the place where the parsing of the token should be done (middleware or fastapi "dependency").

antarest/login/utils.py Outdated Show resolved Hide resolved
antarest/login/utils.py Show resolved Hide resolved
antarest/login/utils.py Outdated Show resolved Hide resolved
antarest/login/utils.py Outdated Show resolved Hide resolved
antarest/login/utils.py Outdated Show resolved Hide resolved
antarest/login/utils.py Outdated Show resolved Hide resolved
tests/core/test_tasks.py Show resolved Hide resolved
tests/core/test_tasks.py Show resolved Hide resolved
@maugde maugde force-pushed the fix/ANT-2543-fix-user-name-display-in-commands branch from 63e4baa to bfcd7aa Compare January 14, 2025 15:36
@maugde maugde requested a review from sylvlecl January 14, 2025 15:46
Copy link
Member

@sylvlecl sylvlecl left a comment

Choose a reason for hiding this comment

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

Thanks for the additional unit test!

Copy link
Contributor

@TheoPascoli TheoPascoli left a comment

Choose a reason for hiding this comment

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

Hello Mauranne, great work ! 👍

@sylvlecl sylvlecl merged commit e70e523 into dev Jan 16, 2025
13 checks passed
@sylvlecl sylvlecl deleted the fix/ANT-2543-fix-user-name-display-in-commands branch January 16, 2025 12:50
smailio pushed a commit that referenced this pull request Jan 17, 2025
A context variable is introduced to hold the use of the current request,
and of the current task.
Can be re-used in the future to avoid passing this context information
as an argument to all methods (for logging, etc).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants