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

add schema validation for serverless >= 2.x #24

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joquijada
Copy link

@joquijada joquijada commented Jul 27, 2021

Looks like as per the documentation here that plugins can define properties in the custom: but not the provider: section of serverless.yml. Moreover plugin properties cannot be in the top level of custom:, but instead must be in a node one level down which is the namesake of the plugin.

Addresses #23

@joquijada
Copy link
Author

Hello @alex-murashkin , are you able to review please? Thanks.

Update documentation to reflect to location of config
@and3rson
Copy link

and3rson commented Apr 26, 2022

Found this PR while searching for the described warning.
However, it seems like this change will be backward-incompatible. What about checking for custom[PLUGIN_NAME] and, if it does not exist, falling back to provider.tracing? i.e.

-    const providerLevelTracingEnabled = (service.provider.tracing === true || service.provider.tracing === 'true');
+    const legacyProviderLevelTracingEnabled = service.provider.tracing?.toString() === 'true';
+    const providerLevelTracingEnabled = (
+     service.custom[PLUGIN_NAME]?.tracing?.toString() === 'true'
+     || legacyProviderLevelTracingEnabled
+   );

(Not an actual code, just a suggestion - I never wrote plugins for serverless)

@andunai
Copy link

andunai commented Dec 13, 2022

Tagging @alex-murashkin - any chance to have this resolved?
We're considering forking this library since there doesn't seem to be any activity over the past 4 years.

and3rson added a commit to and3rson/serverless-plugin-xray that referenced this pull request Dec 13, 2022
@and3rson
Copy link

@joquijada I've applied your PR with some modifications into my fork: https://github.com/and3rson/serverless-plugin-xray
It's available on NPM as serverless-plugin-xray.

@alex-murashkin
Copy link
Owner

Hi everyone, thank you for your contributions! Unfortunately, I've been away from the serverless space for a while, and do not have time to maintain the plugin.
I suggest forking and using a forked npm package instead like @and3rson suggested above.

@joquijada
Copy link
Author

Hi All, Just catching up. I've been away. At work we started migrating to SST. Since this PR was never merged, I forked this repo, created a tarball using npm pack and put it in source control, then I linked to it from the package.json in question using relative paths, since the serverless-plugin-tracing tarball and the project that consumes it are all under the same Git project. I recommend just migrate to SST, and you won't have to worry again about serverless plugins that go stale.

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.

4 participants