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

support running in remote extension host #76

Closed
wants to merge 2 commits into from

Conversation

kalinkrustev
Copy link

@kalinkrustev kalinkrustev mentioned this pull request Dec 14, 2021
5 tasks
@alefragnani
Copy link
Owner

Hi @kalinkrustev ,

Thanks for the PR, but I'm not sure what the effect of this change. And in fact, that's not what #41 is about.

The idea behind #41 is to enable the extension to work on remotes like SSH, WSL, Container, without the need to install the extension on that remote. To accomplish this, a few changes (API and contributing related) needs to be made.

Could you please provide more details about the meaning of this change?

Thank you

@kalinkrustev
Copy link
Author

OK, I am not sure how easy it is to achieve that without installing the extension at the remote. My proposal makes it work when the extension is installed at the remote. The effect of this change is to use the original require, not the webpack one, to read the contents from .jenkinsrc.js on the remote. Without this change, it tries to read it from the webpacked extension, where it obviously does not exist.

@alefragnani
Copy link
Owner

The .jenkinsrc.js file is not bundled with the extension. This is an alternative to the .jenkins file, if you need add some script to provide the URLs, like discovering the current branch to be sent to the Jenkins request. This was created in this PR a few years ago #17 (comment)

If that's not your scenario, there will be no need to install the extension on the remote (once it is updated to work with remotes). The extension should be installed only locally, and it would work correctly on any remote (Container, WSL, SSH, Codespaces).

I mean, if your .jenkinsrc.js file is something like this:

// can also return a promise of required JSON structure
module.exports = [{
    "url": "http://127.0.0.1:8080/job/myproject/",
    "name": "Jenkins Build",
    "username": "jenkins_user",
    "password": "jenkins_password_or_token"
},
{
    "url": "http://127.0.0.1:8080/job/myprojectTests/",
    "name": "Jenkins Acceptance Tests",
    "username": "jenkins_user",
    "password": "jenkins_password_or_token"
}];

You could easily change to the .jenkins format

[
    {
        "url": "http://127.0.0.1:8080/job/myproject/",
        "name": "Jenkins Build",
        "username": "jenkins_user",
        "password": "jenkins_password_or_token"
    },
    {
        "url": "http://127.0.0.1:8080/job/myprojectTests/",
        "name": "Jenkins Acceptance Tests",
        "username": "jenkins_user",
        "password": "jenkins_password_or_token"
    }
]

And it has the same effect.

@kalinkrustev
Copy link
Author

I need it to be .jenkinsrc.js, because I need to put some logic and and not use static URLs. The approach of installing the extension on the remote is just an easier way to achieve the functionality. So this change just makes sure that when using require() to load .jenkinsrc.js it is not trying to load the file from the webpacked output (which is what is currently happening). The eslint extension (also installed in the remote) from which I borrowed the solution is doing basically the same thing, when it tries to load its config files.

@alefragnani
Copy link
Owner

That's really weird the extension to have issues loading .jenkinsrc.js inside on remote, even when installed on remote. It should work just like a local installation. But I could confirm that it does not work. 😕

Using your snippet, it indeed does work, and I'll apply it to this scenario. But in order to keep the extension easier to use (installed locally and working on remotes), other changes are needed (the regular support remotes approach). This will support the default .jenkins file, locally and remotely.

Thank you

@alefragnani
Copy link
Owner

Closing, in favor of #78

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

Successfully merging this pull request may close these issues.

None yet

2 participants