Allow extra properties#7
Conversation
Quick vibe code to close vsoch#6
ax3l
force-pushed
the
custom-extra-prop
branch
from
129398e to
2852d54
Compare
|
Testing in openPMD/openPMD-api#1826 |
vsoch
left a comment
vsoch
left a comment
There was a problem hiding this comment.
Thank you! Some questions and comments.
| # Use empty schema {} which allows any type | ||
| for prop in allowed_props: | ||
| if prop not in schema['properties']: | ||
| schema['properties'][prop] = {} |
There was a problem hiding this comment.
What happens if the property is nested? And where is the full definition of the property?
| allowed_props = [prop.strip() for prop in allowed_props_str.split(',') if prop.strip()] | ||
|
|
||
| if not allowed_props: | ||
| print('❌ ERROR: No valid property names found in allowed_extra_properties', file=sys.stderr) |
There was a problem hiding this comment.
You can just do sys.exit("Message") and it will exit with error and print the message.
| try: | ||
| schema_path = os.environ['SCHEMA_PATH_ESC'] | ||
| final_schema_path = os.environ['FINAL_SCHEMA_PATH_ESC'] | ||
| allowed_props_str = os.environ['ALLOWED_PROPS_ESC'] |
There was a problem hiding this comment.
nit: variable names should not have types.
| | :--- | :--- | :--- | | ||
| | `path` 📍 | Where is your `.zenodo.json`? | `.zenodo.json` | | ||
| | `error_format` 🎨 | `text`, `json`, or `pretty-json` | `text` | | ||
| | `allowed_extra_properties` ✨ | Comma-separated list of extra property names to allow (e.g., `pub_id,custom_field`) | `''` (empty) | |
There was a problem hiding this comment.
Maybe just extra_properties ? I think allowed is implied.
|
|
||
| ### 🔓 Allowing Extra Properties | ||
|
|
||
| Some Invenio instances (like [RODARE](https://rodare.hzdr.de)) require additional properties beyond the standard Zenodo schema. You can explicitly allow these properties using the `allowed_extra_properties` input: |
There was a problem hiding this comment.
What if something is allowed and required? Or requires more than just being in the listing? Notably, we are adding the allowed extra properties, but not defining types, etc.
| FINAL_SCHEMA_PATH="/tmp/modified_schema.json" | ||
|
|
||
| # Use Python to modify the schema and add allowed extra properties | ||
| if ! SCHEMA_PATH_ESC="$SCHEMA_PATH" \ |
There was a problem hiding this comment.
For this block let's:
- Write an actual Python script with argparse
- Run the command here, providing the environment variables
It will be more understandable for the reader developer than as currently done.
| import os | ||
| import sys | ||
|
|
||
| try: |
There was a problem hiding this comment.
LLMs have a tendency to wrap everything in try/except. My preference would be to not do that. We aren't catching any specific error here, especially with the various checks. If there is an error I want it to raise and see it and don't need the additional wrapping.
| if prop not in schema['properties']: | ||
| schema['properties'][prop] = {} | ||
|
|
||
| # Write the modified schema |
There was a problem hiding this comment.
Make sure that comments are useful/meaningful.
2 participants
Quick vibe code to close #6