-
Notifications
You must be signed in to change notification settings - Fork 15
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
🐛 Fix git-based user/email info for aiida profile #78
🐛 Fix git-based user/email info for aiida profile #78
Conversation
@GeigerJ2 thanks for making these fixes! Could you give me a good |
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 a lot @GeigerJ2 for looking into this! It's gone neglected for far too long :)
Unfortunately I think the string wrangling needs a second look - I get this in aiida_config.yaml
:
...
first_name: "'Rok Roškar'"
last_name: "Rok Roškar'"
institution: Renkulab
|
||
project_dir=`pwd` | ||
project_dir=$(pwd) | ||
|
||
cat > aiida_config.yaml <<EOF | ||
store_path: "${project_dir}/repo" |
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 realize this is not what you are addressing in this PR, so it's fine if you take it elsewhere, but repo/
should be added to .gitignore
(and maybe called something more descriptive?)
@@ -44,4 +45,4 @@ export AIIDA_PATH="${project_dir}/repo" | |||
|
|||
# create database, don't start daemon | |||
reentry scan | |||
source aiida-activate aiida_config.yaml -c | |||
source aiida-activate aiida_config.yaml -c |
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.
newline missing
Not sure if you still need this, but I've been using: for testing. |
Hm, that seems weird. There are both single quotes for |
Thanks @GeigerJ2 - how do I verify what was used for |
That's a good question. I thought it would be shown, but also for me, it doesn't. I really think it should, though. We're currently having some discussions about the internals of AiiDA profiles, so I'll bring that up for sure. In the meantime, you could see the I just checked, and also in my case the first (last) name has an extra leading (trailing) |
Each profile can have multiple users with one being the default. You can show them with |
I am not quite sure how the code that parses the git username into a first and last name is supposed to work, but it seems not to be working as the first and last name both contain the entire name (including the problem with the quotes). I think the following should work as intended: email="$(git config user.email)"
user="$(git config user.name)"
first_name=$(echo $user | cut -d ' ' -f1) # Take all characters up until first space
last_name=$(echo $user | cut -d ' ' -f2-) # Take all characters after the first space
cat > aiida_config.yaml <<EOF
profile: "default"
email: $email
first_name: $first_name
last_name: $last_name
institution: Renkulab
EOF You don't need quotes around strings in YAML and think it might be part of the problem of the stray single quotes, but mostly this |
Yeah, frankly, I also didn't fully understand that. I just left it because I thought it was there for a reason. Didn't realize it's that broken, though :D But in the latest version, I also replaced it. It still uses a bash array as before, i.e.: read -ra name_array <<< "$(git config user.name | tr -d "'")"
first_name="${name_array[0]}"
last_name="${name_array[@]:1}" But indeed, using I think the problem with the single quotes, however, is that running |
@GeigerJ2 if the changes here aren't needed lets close this and I will make a new release so that the new templates can be used by everyone. |
Yes, fine for me :) |
This PR provides a first fix for the broken AiiDA template for RenkuLab.
Recently, trying to explore an AiiDA archive from Materials Cloud via RenkuLab created a Jupyter Notebook in which the provided code wasn't working due to a missing AiiDA profile. Manual profile creation via the shell would alleviate the issue, so the Docker image and AiiDA setup itself seem to be (in principle) still fine. As it turns out, though, the
$GIT_AUTHOR_NAME
and$EMAIL
environment variables are unset, which is why the creation of the AiiDA profile on startup via thepost-init.sh
script fails. Now, the relevant information is obtained viagit config
, fixing the current problem.However, the overall template is a bit outdated, using AiiDA v.1.6, and the
aiida-activate
tool, which has not beenchanged in the last 4 years. I have another PR in the pipeline that updates the template, i.e. using AiiDA v2.5,
removing the
aiida-activate
dependency, and providing a new Docker image, but we're still testing some things. It'llcome soon, though :)