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

🐛 Fix git-based user/email info for aiida profile #78

Conversation

GeigerJ2
Copy link
Contributor

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 the post-init.sh script fails. Now, the relevant information is obtained via git 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 been
changed 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'll
come soon, though :)

@rokroskar
Copy link
Member

@GeigerJ2 thanks for making these fixes! Could you give me a good archive_url to test this with?

Copy link
Member

@rokroskar rokroskar left a 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"
Copy link
Member

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
Copy link
Member

Choose a reason for hiding this comment

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

newline missing

@GeigerJ2
Copy link
Contributor Author

@GeigerJ2 thanks for making these fixes! Could you give me a good archive_url to test this with?

Not sure if you still need this, but I've been using:

https://archive.materialscloud.org/record/file?filename=acwf-verification_unaries-verification-PBE-v1_results_gpaw.aiida&record_id=1770

for testing.

@GeigerJ2
Copy link
Contributor Author

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

Hm, that seems weird. There are both single quotes for first_name and only one for last_name? In any case, could you please give it a shot with the latest version on #79, and the URL I commented to see if there's an issue with the created AiiDA profile. In and of itself, AiiDA should handle special characters like "š" just fine (at least I hope so). Thanks!

@rokroskar
Copy link
Member

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

Hm, that seems weird. There are both single quotes for first_name and only one for last_name? In any case, could you please give it a shot with the latest version on #79, and the URL I commented to see if there's an issue with the created AiiDA profile. In and of itself, AiiDA should handle special characters like "š" just fine (at least I hope so). Thanks!

Thanks @GeigerJ2 - how do I verify what was used for first_name last_name? When I do verdi profile show it doesn't show them.

@GeigerJ2
Copy link
Contributor Author

Thanks @GeigerJ2 - how do I verify what was used for first_name last_name? When I do verdi profile show it doesn't show them.

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 verdi profile setup command that was run via the pid file in the .aiida//access/aiida_renku directory, e.g.: /home/jovyan/work/user-name-test/aiida_data/.aiida/access/aiida_renku/18.pid

I just checked, and also in my case the first (last) name has an extra leading (trailing) '. So definitely something's going wrong in the post-init.sh script. Thanks for figuring this out, it might have been difficult to catch otherwise , without the yaml file! I'll resolve that tomorrow.

@sphuber
Copy link

sphuber commented Apr 17, 2024

Thanks @GeigerJ2 - how do I verify what was used for first_name last_name? When I do verdi profile show it doesn't show them.

Each profile can have multiple users with one being the default. You can show them with verdi user list.

@sphuber
Copy link

sphuber commented Apr 17, 2024

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 join_by function from stackoverflow seems to be culpable and simply not working.

@GeigerJ2
Copy link
Contributor Author

GeigerJ2 commented Apr 18, 2024

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 cut might be simpler. Too much bash syntax (e.g. arrays) gives me shivers.

I think the problem with the single quotes, however, is that running git config user.name fundamentally prints the name surrounded by single quotes in the shell on RenkuLab - which it doesn't do on any of the local systems I use. So I guess this ties in with the somewhat different behavior of git now as compared to when the template was first created, similar to the reason why it broke in the first place. I'll update the parsing to YAML here, using your snippet, then this should be ready for merge, and the full update hopefully soon, as well, when @rokroskar gets around to have a final look. Thanks!

@rokroskar
Copy link
Member

@GeigerJ2 this is now already taken care of in #79, correct?

@rokroskar
Copy link
Member

@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.

@GeigerJ2
Copy link
Contributor Author

Yes, fine for me :)

@GeigerJ2 GeigerJ2 closed this May 13, 2024
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