Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Justin Harringa
    @justinharringa:matrix.org
    [m]
    I plan on using the time for review 🙂
    Justin Harringa
    @justinharringa:matrix.org
    [m]
    I won't be able to meet this week unfortunately. I'll be back next week.
    Harshit Chopra
    @arpoch
    @MarkEWaite @rishabhBudhouliya @justinharringa:matrix.org , regarding the openssh key format used for encrypted private key, should we use sshj library for this purpose or making our know implementation for this purpose is useful? Although the latter approach is tedious but could have benefits in long term.
    Mark Waite
    @MarkEWaite
    I generally prefer using someone else's implementation, especially when it seems the library is receiving regular maintenance (https://github.com/hierynomus/sshj/releases). However, if you see sshj as a significant liability or if we discover that it does not work well in the Jenkins plugin, we certainly can consider implementing only what is needed for the use case of the project.
    Harshit Chopra
    @arpoch
    @MarkEWaite , @rishabhBudhouliya , @justinharringa:matrix.org , if migrating the binding to git plugin then we need to explicitly ask for the git version installed on the machine from the user since the isAtleastVersion method has package-private scope, keeping this in mind I have not made any commits for the ssh binding yet.
    Will make the commits for ssh binding once sure in which plugin the binding are to be implemented. Currently the ssh binding is implemented in git client plugin.
    Mark Waite
    @MarkEWaite
    I thought that the git plugin already had a method that would determine CLI git version. We can certainly make that a public method in the git client plugin if that helps with the implementation.
    Harshit Chopra
    @arpoch
    Couldn't find it, making a public method for this purpose will surely help.
    Mark Waite
    @MarkEWaite
    Public method in git client is great with me.
    Harshit Chopra
    @arpoch
    @rishabhBudhouliya the resolveGitTool method resolves/finds Git Tool by name. So if using resolveGitTool for our specific use case then the gitTool name used will be Default as we are not taking any input from the user regarding the git Tool Name so we can't make any assumptions on what name the user will set for the git tool thus we can't supply the gitTool name for resolveGitTool method which comes down to using Default only.
    My implementation gitToolName is type specific, which only matches the type with the type of tool required irrespective of name set by the user. Both the implementation resolveGitTool and gitToolName are somewhat same only differs in the approach of the find the git tool. Also the time complexity of both the impl will be linear once the forNode method used in the gitToolName method is removed since it's not required their. So I think it's better to make a separate method in git plugin under GitUtils class which resolves the git Tool name based on the type rather than name.
    Harshit Chopra
    @arpoch
    @MarkEWaite, should I make a PR regarding the creation of a public method for comparing the cli git version with the required version. I am thinking of adding compareLeastGitVersion method signature in the GitClient interface. Will also create a test case for this change.
    Mark Waite
    @MarkEWaite
    Yes, a PR is the right approach to add a public method. Initially it will need an implementation in CliGitAPIImpl and a no-op in the JGit implementations
    Harshit Chopra
    @arpoch
    I have shifted the impl for compareLeastGitVersion to CliGitAPIImpl class and it seem reasonable as well since it is exclusively for cli git, also I have not added any additional test case but updated the existing assertVersionOutput method to check the compareLeastGitVersion method as well.
    Harshit Chopra
    @arpoch
    PR to resolve the above issue - jenkinsci/git-client-plugin#724
    Harshit Chopra
    @arpoch
    @rishabhBudhouliya , @MarkEWaite , the meeting haven't started yet?
    Rishabh Budhouliya
    @rishabhBudhouliya
    @arpoch I assumed it will begin at 8.30 AM on the CDF zoom account
    Mark Waite
    @MarkEWaite
    Sorry for my delay! Meeting ran longer than I expected
    Harshit Chopra
    @arpoch
    Created the PR for change of implementation from git-client to git plugin [jenkinsci/git-plugin#1104]
    Not added documentation changes in the PR though, I might need some changes, as I have added an input box to retrieve the git tool name from the user so let me know if this servers the purpose as expected, if the these changes are kept then documentation needs work.
    Also this https://www.jenkins.io/doc/developer/plugin-development/incrementals/ page needs improvement will open a PR with the required changes sooner or later.
    Mark Waite
    @MarkEWaite
    That's a great result @arpoch . I look forward to reviewing and interactive testing
    Harshit Chopra
    @arpoch
    I will join in a few mins
    Rishabh Budhouliya
    @rishabhBudhouliya
    I have joined
    Harshit Chopra
    @arpoch
    It says meeting has not started yet
    has the meeting started? I can't join though.
    Rishabh Budhouliya
    @rishabhBudhouliya
    I am not sure if Mark has joined yet, meanwhile I have created a session: https://narvar.zoom.us/j/96596666166?pwd=UzJQYkxxMUJmb0pCMjMya3Bmb1Jhdz09
    Harshit Chopra
    @arpoch
    @rishabhBudhouliya , apologies due my network issue
    Rishabh Budhouliya
    @rishabhBudhouliya
    @MarkEWaite @justinharringa:matrix.org apologies, I forgot to record the meeting. Harshit and I have tried to capture the details of the meeting in the meeting notes as much as we could, let us know if anything needs a discussion
    Mark Waite
    @MarkEWaite
    So sorry! I was playing with my grandchildren and forgot that we had a meeting today! See my comments in the pull request at jenkinsci/git-plugin#1104 . I'll review the meeting notes separately to assure that I'm current on the status.
    Harshit Chopra
    @arpoch
    @MarkEWaite , were should we add doc for git-credentials binding in git-plugin doc?
    Mark Waite
    @MarkEWaite
    In the README.adoc file
    Mark Waite
    @MarkEWaite

    Looking forward to the git credentials binding project office hours in about 30 minutes. I've spent an hour or two testing the most recent build and have good results to share.

    1. So long as I define the correct gitToolName, Pipeline and Freestyle jobs are working great in all the tests that I have run
    2. If I fail to define gitToolName, the job fails (Freestyle and Pipeline)

    Great work @arpoch !

    Mark Waite
    @MarkEWaite
    Shame on me for confusing one meeting with another. Docs office hours starts in a few minutes. Git credentials is tomorrow. Talk to you all tomorrow. Sorry for the mistaken calendar reading
    1 reply
    Harshit Chopra
    @arpoch
    1. If I fail to define gitToolName, the job fails (Freestyle and Pipeline)
    I believe the scenario of leaving git tool name input empty is not a valid behavior thus requires a check to ensure that at-least the git tool input contain Default.
    image.png
    Justin Harringa
    @justinharringa:matrix.org
    [m]
    Hey I'm sorry for the late notice but I have had some things come up and can't make it. Hopefully Mark has you set for the meet
    Mark Waite
    @MarkEWaite
    Meeting notes are in https://docs.google.com/document/d/1gZneYIDWrT5S-1ACG641wfvxs7vnDC0RCYqy-EuuhwY/edit?usp=sharing . Looks good to release a version of git client plugin with the new API and git plugin with the username / password credentials binding implementation
    Mark Waite
    @MarkEWaite
    @arpoch and @rishabhBudhouliya I've investigated further and I believe the logic is now implemented (with tests) that will allow gitToolName to be an optional argument and to use default if an invalid value is provided. See MarkEWaite/git-plugin@4f8e4d8
    Harshit Chopra
    @arpoch
    @MarkEWaite I believe you wanted something like this... for the git tool name selection
    2 replies
    image.png
    Harshit Chopra
    @arpoch
    I have a doubt now. The doFillGitToolItems in GitSCM provides us with names of the git tools available for the user to choose from. Could we use the same logic but return a gitTool instance instead and check it's type and decide on whether to proceed authentication or not, will it still require resolveGitTool method usage. When using resolveGitTool method we check for a node specific gitTool but if the user is the one selecting the gitTool then would it be too assumptious to say that they must have configured it on the agents as well.
    Mark Waite
    @MarkEWaite
    I think it is still best to not require the user select the correct gitTool.
    It helps us that they recommend a git tool and we should prefer that git tool, but I don't think we should fail if they provided a git tool and their sh, bat, or powershell can find a command line git to use
    Rishabh Budhouliya
    @rishabhBudhouliya
    I think the approach Harshit is describing depends on a critical factor:
    Does the doFillGitToolItems provide all the git installations or only the git installations present in the current running node? (be it a controller or an agent machine)
    And I agree with Mark's advice, we should minimise the responsibility given to a user and work for the optimal solution under the given constraints
    Harshit Chopra
    @arpoch

    Does the doFillGitToolItems provide all the git installations or only the git installations present in the current running node? (be it a controller or an agent machine)

    I believe it returns all the git tools listed under Git in Global Tool Config.

    Mark Waite
    @MarkEWaite
    Git client plugin 3.8.0 has been released with the new API for the git credentials binding. Should be able to update the dependency in the git plugin pull request so that it requires a released version. Thanks for your great results @arpoch
    Harshit Chopra
    @arpoch
    Will be late to the meeting by 5-10 mins today
    Harshit Chopra
    @arpoch
    we could start the meeting now
    Mark Waite
    @MarkEWaite
    So sorry that I missed the meeting! I was called into another meeting at a different physical location and didn't have time to notify everyone that I would be away. Will be doing some testing and exploring tomorrow to interactively confirm the most recent changes.
    Rishabh Budhouliya
    @rishabhBudhouliya
    Apologies I missed the meeting too, I wasn't feeling that good this morning. @arpoch did anyone join it?
    2 replies
    Harshit Chopra
    @arpoch
    @rishabhBudhouliya , @MarkEWaite , @justinharringa:matrix.org , created my slides for the presentation, if their is something missing let me.
    https://docs.google.com/presentation/d/1LCH0dXzWka_l-WQ3SVMCXfU7w7jQENXS-bdz2E5GIgU/edit?usp=sharing