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: remove strict pattern on grafanaDependency #280

Closed
wants to merge 1 commit into from

Conversation

darrenjaneczek
Copy link

Copy link
Member

@academo academo left a comment

Choose a reason for hiding this comment

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

You can disable specific validations for your plugin via a configuration file as described here https://github.com/grafana/plugin-validator?tab=readme-ov-file#using-a-configuration-file

The current condition, even though strict, still serves a meaningful purpose for most of the cases, removing it will open the possibility to plugins submitting wrong versions (that are not as complex as your specific case). Keep in mind we run the validator for community submissions and in the review process we can manually decide to overlook an specific problem such as this if the author explains the problem or we note it.

Additionally, this file is a fallback for the validator, the validator always fetches the latest schema from the grafana/grafana repository https://github.com/grafana/grafana/blob/ad3fc957558022dc4199a6c060f06bf038c7d7bd/docs/sources/developers/plugins/plugin.schema.json#L

Alternatively maybe you can propose an updated regex that covers the condition instead of removing it?

@darrenjaneczek
Copy link
Author

You can disable specific validations for your plugin via a configuration file as described here https://github.com/grafana/plugin-validator?tab=readme-ov-file#using-a-configuration-file

It is not clear how I can disable this specific check for the grafanaDependency entry.
Until I can, my only other options are to disable this check entirely, or change my version range to something simpler like >=10.4.11 to comply.

Additionally, this file is a fallback for the validator, the validator always fetches the latest schema from the grafana/grafana repository https://github.com/grafana/grafana/blob/ad3fc957558022dc4199a6c060f06bf038c7d7bd/docs/sources/developers/plugins/plugin.schema.json#L

That is definitely a problem here.

Alternatively maybe you can propose an updated regex that covers the condition instead of removing it?

I don't believe a regex is a the best tool for parsing a valid semver version range. It would be difficult (if possible?) to derive an exhaustive pattern that covers all cases, and it would be repetitious and hard to read as a human. I gave it a time-boxed try, and failed to find something satisfactory.

In #279, I suggest an alternative check which makes use of the semver library itself. I don't think that's an option though if we're just checking through the schema.

@darrenjaneczek
Copy link
Author

@academo, I have closed this PR since based on what you said it doesn't disable the pattern (due to it being expressed in grafana/grafana). This is still a unresolved problem for our project. See #279 for more detail.

You can disable specific validations for your plugin via a configuration file as described here https://github.com/grafana/plugin-validator?tab=readme-ov-file#using-a-configuration-file

Looking at the documentation, it is not clear how I can disable this specific check for the grafanaDependency entry. Any help would be appreciated.

@darrenjaneczek darrenjaneczek deleted the dj/remove-grafanaDependency-pattern branch November 18, 2024 18:41
@academo
Copy link
Member

academo commented Nov 27, 2024

hey @darrenjaneczek my apologies for responding so late to your message

Looking at the documentation, it is not clear how I can disable this specific check for the grafanaDependency entry.

You can disable the whole metadataschema validator. like this

analyzers:
  metadataschema:
    enabled: false

you are correct this will disable the whole schema validation not only the specific one for grafanaDependency but maybe this will help with your problem of having errors in your CI?

@darrenjaneczek
Copy link
Author

Disabling the analyzers seems to be the best solution for our project:

global:
  enabled: true
  severity: warning
  jsonOutput: false
  reportAll: false
analyzers:
  license:
    enabled: false
  # The metadatavalid metadataschema checkers are too strict for our plugin
  # Our grafanaDependency version range is complicated,
  # and our plugin ids have internal dashes.
  metadatavalid:
    enabled: false    
  metadataschema:
    enabled: false      

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🚀 Shipped
Development

Successfully merging this pull request may close these issues.

2 participants