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

feat: load tool(s) via URL #121

Merged
merged 12 commits into from
Nov 29, 2023
Merged

Conversation

henryiii
Copy link
Collaborator

@henryiii henryiii commented Oct 23, 2023

This adds --tool <name>=<url>#<pointer> support. I've also added basic support for pyodide (untested, probably can add eventually), since loading URLs is different there.

@henryiii henryiii force-pushed the henryiii/feat/urls branch 2 times, most recently from f11ee0c to 1474016 Compare November 1, 2023 21:36
@henryiii henryiii force-pushed the henryiii/feat/urls branch 3 times, most recently from 4ba6a65 to a7b9b21 Compare November 14, 2023 21:58
@henryiii henryiii force-pushed the henryiii/feat/urls branch 2 times, most recently from 13ea848 to 89d7925 Compare November 20, 2023 16:47
@henryiii henryiii changed the title WIP: load tool via URL feat: load tool(s) via URL Nov 20, 2023
@henryiii henryiii marked this pull request as ready for review November 20, 2023 20:28
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Copy link
Owner

@abravalheri abravalheri left a comment

Choose a reason for hiding this comment

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

Thank you very much for working on this @henryiii.

I wonder if it would make more sense for the API layer to be more focused on complex types instead of primitives and let the CLI layer to do the lifting.

For example, would some of the following alternatives work as Python API?

Validator(
    load_tools={
       "cibuildwheel": "https://json.schemastore.org/cibuildwheel.json#/properties/tool/properties/cibuildwheel",
    }
)

# OR

Validator(
    tools=[
        RemotePlugin("cibuildwheel", "https://json.schemastore.org/cibuildwheel.json#/properties/tool/properties/cibuildwheel"),
        # Would require `RemotePlugin.load()` or something similar
    ]
)

# OR

Validator(
    plugins=[
        PluginWrapper(...),
        RemotePlugin("cibuildwheel", "https://json.schemastore.org/cibuildwheel.json#/properties/tool/properties/cibuildwheel"),
        # Would require `RemotePlugin.schema` to lazily trigger/cache the download
    ]
)

src/validate_pyproject/api.py Outdated Show resolved Hide resolved
src/validate_pyproject/errors.py Outdated Show resolved Hide resolved
Co-authored-by: Anderson Bravalheri <andersonbravalheri+github@gmail.com>
@henryiii
Copy link
Collaborator Author

henryiii commented Nov 28, 2023

I basically followed your third suggestion, though I didn't implement lazy loading (it would complicate the implementation quite a bit, and since id has to be loaded pretty early, I don't think there's a benefit; I did cache though). I also added a helper class method to easily handle the tool=url form since it's used multiple places.

@henryiii
Copy link
Collaborator Author

I also had to add extra_plugins, since setting a plugin replaces ALL_PLUGINS. Ideally, that would be an all_plugins: bool parameter and one plugin list. Can change the API to that if you want.

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii henryiii force-pushed the henryiii/feat/urls branch 2 times, most recently from ad51df0 to f59a976 Compare November 28, 2023 17:17
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Copy link
Owner

@abravalheri abravalheri left a comment

Choose a reason for hiding this comment

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

Thank you very much, @henryiii.

This looks good. If you are happy with it, please feel free to merge.

I will be away from a proper computer in December, and probably less responsive. But if at any point you want to cut a release, please feel free to do so. I believe that it can be done via the github webUI, and then the CI should take care of it...

@henryiii henryiii merged commit 2c03720 into abravalheri:main Nov 29, 2023
29 checks passed
@henryiii henryiii deleted the henryiii/feat/urls branch November 29, 2023 14:38
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.

None yet

2 participants