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

Schema wants OTP to be a string, but it's really a number #351

Closed
Phault opened this issue Aug 7, 2022 · 4 comments · Fixed by #353
Closed

Schema wants OTP to be a string, but it's really a number #351

Phault opened this issue Aug 7, 2022 · 4 comments · Fixed by #353
Assignees
Labels
good first issue Good for newcomers

Comments

@Phault
Copy link

Phault commented Aug 7, 2022

Providing the OTP by passing it through the CLI fails as the value provided is coerced into a number, rather than staying a string like the executor schema expects.

>  nx run utilities:deploy-npm --otp=634119
Property 'otp' does not match the schema. '634119' should be a 'string'.

As a workaround, I've patched the schema to expect a number and it works just fine.

Using Nx 14.3.6 and ngx-deploy-npm 4.1.2.

@dianjuar
Copy link
Member

dianjuar commented Aug 8, 2022

I'm not entirely familiar with the OTP parameter. Is the OTP parameter always a number?


As a workaround, I'll suggest doing the following:

- nx run utilities:deploy-npm --otp=634119
+ nx run utilities:deploy-npm --otp='634119'

@dianjuar
Copy link
Member

dianjuar commented Aug 8, 2022

If the right path is changing the OTP type to a number, not a string, we are open to receiving the contribution.

A good starting point is changing the type of the OTP parameter on:

@dianjuar dianjuar added the good first issue Good for newcomers label Aug 8, 2022
@Phault
Copy link
Author

Phault commented Aug 9, 2022

I'm not entirely familiar with the OTP parameter. Is the OTP parameter always a number?

Not entirely sure, but it seems like it is. Still, string seems more correct for a password and it's also what npm themselves list the parameter as, so I think the proper fix would be Nx making their type conversion for CLI parameters smarter. I've created nrwl/nx#11519 in hopes this will get fixed.

For now I think the proper workaround would be accepting both number and string. I will create a PR shortly.

As a workaround, I'll suggest doing the following:

- nx run utilities:deploy-npm --otp=634119
+ nx run utilities:deploy-npm --otp='634119'

This unfortunately makes no difference, it still gets converted to a number by Nx. :(

@dianjuar dianjuar self-assigned this Aug 10, 2022
@dianjuar
Copy link
Member

dianjuar commented Aug 12, 2022

I was able to make it run with:

- nx run utilities:deploy-npm --otp=634119
+ npx nx deploy node-lib1 --otp "'634119'"

But I still think it can be improved by changing the type

dianjuar pushed a commit that referenced this issue Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants