-
Notifications
You must be signed in to change notification settings - Fork 383
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(credential_manager): allow users to overwrite location of session file #2976
feat(credential_manager): allow users to overwrite location of session file #2976
Conversation
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.
Thank you for this contribution 😊 LGTM!
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.
There were 2 pycodestyle failures. Could you please fix those. After it I will merge this.
@@ -21,7 +21,8 @@ | |||
from codechecker_common.logger import get_logger | |||
from codechecker_common.util import load_json_or_empty | |||
|
|||
from codechecker_web.shared.env import check_file_owner_rw, get_password_file | |||
from codechecker_web.shared.env import check_file_owner_rw, get_password_file, \ |
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.
Failed on pycodestyle:
client/codechecker_client/credential_manager.py:24:80: E501 line too long (80 > 79 characters)
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.
Wasn't aware that the line length was 79, not 80. Thanks for spotting!
Fixed
CodeChecker. By default CodeChecker will use | ||
'~/.codechecker.session.json'. This can be used if | ||
restrictive permissions forbid CodeChecker from creating | ||
files in the users home directory (e.g. in a CI environment). |
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.
Failed on pycodestyle:
client/codechecker_client/cmd/store.py:137:80: E501 line too long (80 > 79 characters)
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.
Fixed
…n file Some CI environments are not allowed to create files outside of their designated workspace. This might include the home directory. This commit allows users to overwrite the hardcoded path and name of the session file by setting the CC_SESSION_FILE environment variable.
Here is an overview of what got changed by this pull request: Issues
======
+ Solved 1
- Added 1
See the complete overview on Codacy |
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.
Thanks 😊
Some CI environments are not allowed to create files outside of their
designated workspace. This might include the home directory.
This commit allows users to overwrite the hardcoded path and name of
the session file by setting the CC_SESSION_FILE environment variable.
I didn't know where - if any - to put tests. If required, please tell me what kind of tests you'd like to see and where I should put them. I only tested locally and in our CI environment and it works as expected.