-
Notifications
You must be signed in to change notification settings - Fork 50
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
add endpoint information at startup. #169
add endpoint information at startup. #169
Conversation
Can we put this info in the log.json, that will be more helpful I think. Something like
as the first entry and then the test logs? |
Adding into |
Yes that is true. But is that a problem? I mean while parsing logs we can start from second entry. Also, we can add an entry called timestamp like
It will help keep track of multiple |
You can skip first line. This needs to be documented. According to JSON it is multi-lined document. This requires more discussion and I would recommend your requirement needs to be fixed in different PR |
Okay I will create an issue with the details. |
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.
Seems like it would be good to separate sdk level "detail" logs from run summary logs - the env flags used by Mint would fall in the latter category along with details like average runtime for the sdk.
echo "SECRET_KEY: ***REDACTED***" | ||
echo "ENABLE_HTTPS: $ENABLE_HTTPS" | ||
echo "SERVER_REGION: $SERVER_REGION" | ||
echo "MINT_DATA_DIR: $MINT_DATA_DIR" |
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.
Is there any way for us to pass MINT_DATA_DIR? If not, should we log it here?
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.
I see that the value for MINT_DATA_DIR can be passed via environment variable.
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.
Yes. for non-docker env i.e. when we run it by hand
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.
LGTM
No description provided.