Pin aiidalab version

[edan-bainglass]

Pinning aiidalab==26.05.2 to get the new schemas including the new citations field.

[edan-bainglass]

@yakutovicha we pin to aiidalab[registry]==24.9.0. This won't catch the new citation handler introduced in aiidalab/aiidalab#488. What's the policy here? Make a new AiiDAlab release and bump the pin here?

[yakutovicha]

What's the policy here? Make a new AiiDAlab release and bump the pin here?

yes

[danielhollas]

What's the policy here? Make a new AiiDAlab release and bump the pin here?

I just release 26.2.0 so you should be able to bump it here. :-)

[danielhollas]

Unfortunately the registry build is still failing even after the aiidalab update. This will need probably more work on the aiidalab package side, probably adding citation field in the Jason schema? I am not very familiar with that code.

[edan-bainglass]

Unfortunately the registry build is still failing even after the aiidalab update. This will need probably more work on the aiidalab package side, probably adding citation field in the Jason schema? I am not very familiar with that code.

I see. Indeed, aiidalab/registry/schemas needs an update to include the new key. A bit convoluted that the schema of the registry resides in the AiiDAlab repo. TBD perhaps. Anyhow, I'll open a PR there to update the schema. Once approved/merged, I guess we'll need to release AiiDAlab again and bump here.

[edan-bainglass]

Hmm, I'm not sure why this isn't working. I temporarily point to my fix branch in aiidalab as a dependency, where the citations key is clearly defined. The branch seems to be correctly picked during installation, but the schema validation still fails. I'm missing something. @danielhollas mind having a look?

[danielhollas]

I don't see anything obviously wrong unfortunately. I'd recommend running the build locally and tracing where is the schema validation happening (and where does it actually get the schema).

[edan-bainglass]

I don't see anything obviously wrong unfortunately. I'd recommend running the build locally and tracing where is the schema validation happening (and where does it actually get the schema).

Update

In apps.schema.json, we pin the schema version to the AiiDAlab version

{
    "$schema": "http://json-schema.org/draft-07/schema#",
    "$id": "https://raw.githubusercontent.com/aiidalab/aiidalab/v21.10.0/aiidalab/registry/schemas/apps.schema.json",
    ...

Unclear why this was done. Updating the version gets us through the current validation error, but then runs into another validation error that is unfortunately too vague:

Error: Failed to resolve schema JSON reference: Extra data: line 1 column 4 (char 3)

Maybe try to run with `--mock-schemas-endpoints` ?

--mock-schemas-endpoints does nothing 🤷🏻‍♂️

Trying to debug...

[edan-bainglass]

@danielhollas I realize that maybe we don't need to implement any of this here. I see that the QE app has an aiidalab entry in its setup.cfg that provides the metadata for the registry. We can then add the citations field there. The only potential update here would be to ensure that the URL tag catches the latest version of the app (it should?). I will open a PR in the app to add this.

[danielhollas]

Oh, very good point! Indeed updating in QeApp should be done. I guess updating it also here would have the advantage that it would (maybe??) apply to all versions? But unclear if we care.

[danielhollas]

Oof, that version pin is unfortunate. :-( you'll now run into all the unrelated errors that were merged in the past and not noticed because of it. :-( as you can see, this part of the stack is not touched often (for understandable reasons)

[edan-bainglass]

Oof, that version pin is unfortunate. :-( you'll now run into all the unrelated errors that were merged in the past and not noticed because of it. :-( as you can see, this part of the stack is not touched often (for understandable reasons)

Yeah man, this part sucks. But let's just get it through for now (not this PR, the one in AiiDAlab to update the schemas). Then we can discuss.

[edan-bainglass]

@danielhollas so I think we might want to be careful here. Currently, for a nicer format, the citation is an HTML string (see result in PR description). But to correctly render the citation as HTML, we would need to do {{ citation | safe }}, which isn't actually safe - in principle, someone can register an app with citation metadata that injects unwanted HTML. A more robust system can be introduced, but it may reduce flexibility (we'd have to enforce citation format).

The easy out is to reduce citations to simple strings, but that degrades user experience, as it will not support links (e.g., to papers). Maybe a compromise could be a string citation + a citation string URL (to be safely rendered in an anchor tag).

Thoughts?

[edan-bainglass]

@danielhollas so this again will fail the tests for the same reason, but we now know it works. All that remains is agreeing on the pinning strategy (aiidalab/aiidalab#505). Note that this PR reduced down to just an aiidalab version pin update, as the citation is fully handled on the QE app side.

[danielhollas]

Thanks, LGTM, feel free to go ahead with this once you make a patch release of aiidalab.