Skip to content
This repository was archived by the owner on Oct 24, 2025. It is now read-only.

Fix runtime not being python#745

Closed
mklenbw wants to merge 6 commits intoserverless:masterfrom
mklenbw:741-non-python-runtime
Closed

Fix runtime not being python#745
mklenbw wants to merge 6 commits intoserverless:masterfrom
mklenbw:741-non-python-runtime

Conversation

@mklenbw
Copy link
Copy Markdown
Contributor

@mklenbw mklenbw commented Nov 28, 2022

When runtime is not set to python don't use it as pythonBin.

Closes: #741

@pgrzesik
Copy link
Copy Markdown
Contributor

Thanks @mklenbw - sorry for not responding sooner, but I've been very busy lately, I'll try to review this PR by the end of the week

Copy link
Copy Markdown
Contributor

@pgrzesik pgrzesik left a comment

Choose a reason for hiding this comment

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

Hey @mklenbw, thanks a lot for your patience, I'm really sorry that I wasn't able to respond sooner to your PR. Do you think you could introduce a test case that covers this case? Please let me know what do you think 🙇

@mklenbw
Copy link
Copy Markdown
Contributor Author

mklenbw commented Jan 9, 2023

Sure. I'll add it right away and text you.

@ShantanuNair
Copy link
Copy Markdown

+1 Also facing this issue. Appreciate the PR!

@mklenbw
Copy link
Copy Markdown
Contributor Author

mklenbw commented Jan 27, 2023

@pgrzesik
Sorry i forgot to push it. Now it's there. Hope this helps.

@pgrzesik
Copy link
Copy Markdown
Contributor

Hey @mklenbw, thank you so much 🙇 Unforutnatelly it looks like formatting is failing, could you please check that?

@mklenbw
Copy link
Copy Markdown
Contributor Author

mklenbw commented Feb 3, 2023

@pgrzesik Got it fixed. Forgot to run prettier. You should consider including husky.
There's another problem with the unit-tests. I guess it was the node16.x runtime (might fail on node14 and node12).
The node12-test should be removed as it's EOL since 30.04.2022.

Comment thread test.js Outdated
@pgrzesik
Copy link
Copy Markdown
Contributor

pgrzesik commented Feb 6, 2023

Hey @mklenbw - thank you, there's a leftover console.log that might be messing up the tests. As for Node12 - modernize the CI matrix on my list of things to do soon 👍

@mklenbw
Copy link
Copy Markdown
Contributor Author

mklenbw commented Feb 7, 2023

@pgrzesik
Oh sorry. Never worked with tape before and needed some visuals ;-). Thanks for looking around.

@mklenbw
Copy link
Copy Markdown
Contributor Author

mklenbw commented Feb 8, 2023

@pgrzesik
I'm sorry. I really tried fixing this, but the most test-requirements are so old that my company doesn't allow installation or requirements like perl are just undocumented and unnecessary, that i'm wasting to much time on this.
That means i will quit working on this PR and just take the plugin as broken and outdated as it is.

If someone wants to continue this topic please take my code and get it to work. I would love to see this masterpiece of a plugin in it's full glance again.

@pgrzesik
Copy link
Copy Markdown
Contributor

pgrzesik commented Feb 8, 2023

No worries, sorry that it became an issue and thanks for all the effors @mklenbw 👍 I'll try to wrap this up over the weekend. I'm trying to keep the lights on with this particular plugin but unfortunately my time has been very limited lately.

@harry-isaac
Copy link
Copy Markdown

+1 for would love to see this fixed

@jameskbride
Copy link
Copy Markdown
Contributor

Picked up where @mklenbw left off, see #773 .

@pgrzesik
Copy link
Copy Markdown
Contributor

pgrzesik commented Aug 1, 2023

thanks @jameskbride - I'll close this one and respond in other PR

@pgrzesik pgrzesik closed this Aug 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plugin destroys deployments with provider.runtime other than python.

5 participants