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.

Sign up for GitHub

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

Jump to bottom

Using the latest packaging version check #103

Open
unkcpz opened this issue Dec 8, 2022 · 6 comments · Fixed by aiidalab/aiidalab#451 · May be fixed by #138
Open

Using the latest packaging version check #103

unkcpz opened this issue Dec 8, 2022 · 6 comments · Fixed by aiidalab/aiidalab#451 · May be fixed by #138

Comments

@unkcpz
Copy link
Member

unkcpz commented Dec 8, 2022

Update the packaging to 22.0 will cause the issue of parsing the version that stable is not a valid version tag. We need take a close look at which app using that and ask the app developer to remove the tag.
@unkcpz unkcpz mentioned this issue Dec 8, 2022
Merged
@danielhollas
Copy link

danielhollas commented Feb 20, 2023 •
edited
Loading

According to the docs, we actually tell the users to use the "stable" classifier in the aiidalab section of setup.cfg, so this is a bug on our side. https://aiidalab.readthedocs.io/en/latest/app_development/publish.html#id3

EDIT: That comment was wrong.

@danielhollas danielhollas mentioned this issue Feb 20, 2023
Closed
@danielhollas
Copy link

danielhollas commented Jun 24, 2024 •
edited
Loading

The bug is coming from inside the house 😱

In AWB repo we have a tag stable, and since we use tags as versions, this breaks with the new packaging which is more strict about version format. The tag stable points to the same commit as v1.3.2. @yakutovicha since you created the tag, is it okay to delete it?

In any case, we need to make the code more robust against invalid versions, otherwise any app pushing an invalid tag will break us.

EDIT: Please don't delete the tag yet, I will use it to test the fix.

@danielhollas
Copy link

Here's the full stacktrace, which is different from aiidalab/aiidalab#339, so the fix will need to be applied in a few different places.

  File "/home/hollas/atmospec/aiidalab-registry/.venv/bin/aiidalab", line 8, in <module>
    sys.exit(cli())
             ^^^^^
  File "/home/hollas/atmospec/aiidalab-registry/.venv/lib64/python3.12/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/hollas/atmospec/aiidalab-registry/.venv/lib64/python3.12/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/home/hollas/atmospec/aiidalab-registry/.venv/lib64/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/hollas/atmospec/aiidalab-registry/.venv/lib64/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/hollas/atmospec/aiidalab-registry/.venv/lib64/python3.12/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/hollas/atmospec/aiidalab-registry/.venv/lib64/python3.12/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/hollas/atmospec/aiidalab-registry/.venv/lib64/python3.12/site-packages/aiidalab/__main__.py", line 691, in build
    build_registry(
  File "/home/hollas/atmospec/aiidalab-registry/.venv/lib64/python3.12/site-packages/aiidalab/registry/web.py", line 89, in build
    apps_index, apps_data = generate_apps_index(
                            ^^^^^^^^^^^^^^^^^^^^
  File "/home/hollas/atmospec/aiidalab-registry/.venv/lib64/python3.12/site-packages/aiidalab/registry/apps_index.py", line 75, in generate_apps_index
    app_data = _fetch_app_data(
               ^^^^^^^^^^^^^^^^
  File "/home/hollas/atmospec/aiidalab-registry/.venv/lib64/python3.12/site-packages/aiidalab/registry/apps_index.py", line 49, in _fetch_app_data
    latest_version = sorted(app_data["releases"], key=parse, reverse=True)[0]
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/hollas/atmospec/aiidalab-registry/.venv/lib64/python3.12/site-packages/packaging/version.py", line 56, in parse
    return Version(version)
           ^^^^^^^^^^^^^^^^
  File "/home/hollas/atmospec/aiidalab-registry/.venv/lib64/python3.12/site-packages/packaging/version.py", line 202, in __init__
    raise InvalidVersion(f"Invalid version: '{version}'")
packaging.version.InvalidVersion: Invalid version: 'stable'

@unkcpz
Copy link
Member Author

unkcpz commented Jul 1, 2024

If it is comes from inside from app, I think it would be nice if we can pop some nice error message and tell which app the problem is coming from. Is it doable?

@danielhollas
Copy link

Yes. I am cooking up a fix, but it is not so simple since it needs to be fixed in multiple places. I have a branch locally.

I think the only sane way is to simply skip invalid versions, since we don't have control over tags on app repositories. We also need to clearly document that the version need to adhere to the Python version specification.

@danielhollas danielhollas mentioned this issue Aug 14, 2024
Merged
danielhollas added a commit to aiidalab/aiidalab that referenced this issue Aug 28, 2024
@danielhollas
Skip invalid app versions (#451)
dbfb853 
The AiiDAlab App versions relies on Git tags. 
However, we have an implicit assumption that the tags are a semantically valid versions,
 specifically versions valid according to PEP440. 
Since we do not have control over the apps, this cannot be enforced in any way.
New versions of `packaging.parse()` function that we use
all over the place started to throw `InvalidVersion` exception
when given an invalid version, which lead to crashes
described in #339 and aiidalab/aiidalab-registry#103.

I went over the all instances of packaging.parse
and made sure the call sites are safe. Finally, this allows us
to unpin the `packaging` dependency.

* Skip invalid versions in _AiidaLabApp.releases
* Show test durations
* Add unit tests for sort_semantic, add 'reverse' parameter
@danielhollas danielhollas linked a pull request Sep 2, 2024 that will close this issue
Open
@danielhollas
Copy link

The fix for this was merged in aiidalab/aiidalab#451, now waiting for #138 to get merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@unkcpz @danielhollas