 All right, welcome, everyone. It's June 2nd, 2023, the start of phase one. This is GitLab plug-in modernization. So topics we had, a previous action item, harsh, creating a unified modeling language diagram of the webhook with help from Fram. Chris, to check if the Git have action added by Basel is working as expected. Chris, I think that one's done. Yeah. Yeah, as far as I could tell it's been done. And then harsh create a snapshot of version six dot acts to publish an artifact or harsh you want to report on either of those two action items. Like the, the UML diagram thing we had, I had a meeting with Fram and we decided on some things and I was, I was going to work on it but then the review got messy and like the PR got messy and I am totally focused on that right now. Like, I'm going to shift it next next week and regarding the snapshot version. It's also going to be next week like my focus is going to be that PR as soon as soon as it gets merged, then I'll start working on the webhook thing. We will discuss about that and that review that Basel did. That was a great review. That was an eye opener. Great. Thank you. Well, and I like, I like the focus on the code. Good for you. That's, that's, that's good choice. All right, so, so then do we want to put an agenda topic that is discussion on the, on the review and its results. Okay, are there any specific topics that are on your mind, harsh in terms of, hey, I'd like to ask about this or ask about that. Yeah, related to the auto detection thing like Basel said to keep it like keep the thing alive, like, and I don't have any problem with that but I was trying to understand the auto detection things. I had some questions related to it like why are we iterating to all the API versions when we know that if the user is not using like the user is not specifying whether to use version three or version four, then we can already use the version for the user like why are we iterating to all the API versions and then checking if it's working or not like isn't it useless kind of feature. The reason that it's iterating is if you're using a very old version of GitLab that does not support the newer API is designed so that you could use that older version without having to explicitly configure Jenkins to talk to it with an older API. So, I think, let me give you a specific example for GitLab before it was, I'm just looking up the API history. Yeah, so the, the GitLab API v3 was introduced sometime around 2017. And, okay, I can't I can't find the version that was introduced in but or the version that v4 was introduced in but the idea was at some point in time. There would have been a GitLab release that only supported v3 API and not the v4 API. It would have been an old version at that time, but not old enough that we wouldn't want Jenkins users to be using it. That we wouldn't want to support Jenkins users who are using it. And so in that case, that's what this auto detection feature is designed for us because without the auto detection. If we tried to connect using the latest API version that we know how to speak that might be too new for an older GitLab server. So the idea was that it would, it would gracefully fall back to an older API version until it found one that both sides can speak. I mean this is the same kind of auto negotiation that Ethernet does, like when you plug in an Ethernet cable. It tries to, like I have 100, I have a gigabit Ethernet switch, and you know if you plug in, you know, if you plug in an Ethernet cable, it'll do this auto negotiation where it attempts to determine what speed both sides of the Ethernet connection can communicate at and chooses the lowest common denominator. So this is the same exact type of protocol here. And that would be useful in the future if they introduced API version five. So you'd imagine that there would be still a lot of GitLab servers that only can speak version four. So that's, that's kind of why I think we should keep this feature because, and I did look up whether they were ever going to have a version five and I did see some discussion about it, although it doesn't seem to be released yet. But it's, yeah, it seems like they are potentially going to have a version five in the future. Yeah, so the whole crux is Jenkins and GitLab both should support like the Jenkins instance and the GitLab instance used by the users, both should support the version, the API version that we are trying to use. And that's why we are iterating through all the API versions and seeing if both of them match. Yeah, yeah, if we're, yeah, if we try to talk to the server using the language that's too new for it to speak, you know, it'll just reject the request, and that that auto detection codes just looping around until it finds an acceptable version. Okay, now, the next question that I had was for the ordinal thing. So, like I saw that the client builders were already sorted and the ordinal what the ordinal thing was trying to do was, it was trying to see whether the ordinal values are different or not, and also trying to compare the client builder IDs so I couldn't really understand because the ordinal values are always going to be different like for version, like for auto detection it's going to be zero for for version three and version four it's going to be one and two respectively. Because they are always, the ordinal value is always have a difference like if the difference is non zero, then it will own, it will only compare on the basis of IDs like version three or version four. So it's never going to use the ordinal thing. I couldn't really understand what was that for. Yeah, I mean I, I don't, I don't know how good the original code was but what I, what I imagine the intention was is to be able to simply sort out the list in some kind of order, because it implements some kind of. Yeah, it is already sorted actually the client builders builders are already sorted, and the ordinal was used as a another layer to sort it more for some reason I don't know. They were trying to compare like if the two client builders are there with the same ID. Like, no with the same ordinal value then the IDs will be compared, but it cannot, it is not possible right we cannot have same ordinal value they will always have some difference. It may very well be overly complicated and or useless I wouldn't be surprised if the original code had some flaws like that, but I haven't looked at it very closely. Anyway, if you, if you think that it could be simplified that seems fine to me. Can I remove that because I don't really find any use case for that. I guess I mean if it really is. Yeah, if it really is over if it really is pointless then, then sure. I haven't, like I said I haven't looked at it too closely but yeah in general if there's, if there's no reason to keep something then then it's fine to remove it. And then I'll, then I make the PR then I'll write it on in the common section so that you can see through it maybe I miss something or something like that, that shouldn't happen. But still, like so yeah, the next thing is, what was that to build the GitLab API, like I what what I was trying to do when I removed the version three was I was trying to create the GitLab API instance in the itself, but now as we need the version three and version four, like some notion of the version three and version four in the code. So what I will be doing is I'll be using the client builders, like in the auto detection client builder what they did was they built the client using the client that was already the client was created in the GitLab plugin code base but now we will be using the GitLab API. So what I will do is I will build the client using the GitLab API in the client builder itself, and the client will because we have the URL and the token in the client in the client builders. So is that a is that a correct approach according to you because I didn't really find anything flawed with it. Yeah, that's fine. Yeah, I mean this is just details. It's just implementation details but if you want to make it really elegant and keep the diff small, you could use the same class name as before like the whatever it was called the auto detecting client builder but you know instead of implementing that old interface that we don't need to implement anymore. It could just be yeah it could just be like a plain old Java class that returns the objects of the GitLab API objects that we need. So, no it has to extend the GitLab builder client because we need to build the client right so it has to be extending that but yeah we don't need to implement those proxies and stuff like that. Right that's right that's right. I think you're on the right track. Yeah, I'm almost but I'm still like, I got underconfident after that PR I wanted to check everything that everything should be fine before I make the PR I don't want to mess up. I don't have to waste my time. No, it's okay. No, I think you're on the right track. I actually wrote the code for it like I actually completed the code for keeping the version three intact and I was interactively testing it but my GitLab got top so maybe I can screen share and discuss the code with you before I make the PR so that it doesn't get messy again. Okay. Sure, let me I'll stop screen sharing and you can start. Oh, system settings and what just wait a minute guys. Oh, I have to quit and reopen zoom so I'll be joining the meeting again. Well, welcome to Mac OS. Oh, you should see how bad Linux is for any of this kind of stuff. Oh, is it is it bad as well I haven't I haven't run. I can't even use my, my Jabra headset with Firefox at all. But it works fine and zoom. So that's one of those dark places where user interface devices are on on on the various operating systems what a black hole I don't envy Microsoft or Apple or anybody who has to deal with. Oh, it's Bluetooth, except it's not quite Bluetooth or it's it's almost Bluetooth does that count. And one of the developers at Oracle and Twitter about maintaining Solaris for new hardware. He said luckily it's a lot easier to keep up with server hardware than it is with desktop hardware. I don't know I've got, I've got a son in law who does controller chips for disk drives and his world. Yeah. Wow. That's already great. Super harsh let's see. Oh, is it visible. Yes it is. Yep. So let me explain what what I have to do, like the best you said about the cost data thing like where is my beautiful review that I got. Yeah this exported thing you were having doubted at this isn't just a temporary change that I'm doing like the merge request thing that I did. This is very much temporary. Where is my course. Yeah, related to removal of this thing. Like it's a temporary thing that I'm doing when I'll be implementing the webhook thing then I'll be separating out the cause data according to the events and then I'll make sure that this thing is inside the merge request events cause data. This is just temporary just to make the plug in work and we have to and just to make sure that the step by step migration is taking place correctly. So I don't think so there is much of a problem here. That sounds fine. In that case would be in that case, why don't we comment it out and put it to do that says this needs to be commented. The plugin will not compile right. Well what I mean is. Oh, oh sorry. Is our in order in order is the temporary code the deletion of this or the addition of it. Like it's deletion of this. Right, right. So, since, since we're deleting something that we that we want to eventually keep in the finished product, what my idea was is that instead of deleting it from the diff, where we could, you know, easily forget about it. We could comment it out instead. And that way it won't, it won't be considered part of the source code. And in that comma we could also write it to do that's something like, you know, figure out how to retain this in the future. Yeah, and the only reason great idea, the only reason for doing that is so that we don't forget about it. And also for, for other people who are reading this to understand more clearly that this is only be removed temporarily, rather than permanently. So I'll write the comment about this, explaining this in the thing, and they are sorry about not explaining it it was it was just a draft PR that's why I did that, like I was not expecting it. Sorry for that you have. The idea is we do want this to get merged into the project branch once, you know, because like, this is incremental progress towards getting the project lead it so that I mean that's why I, that's why I lowered the requirements for the project branch to reduce. You know having to compile the tests having to run the tests. The idea would be to get a clean build of this. And, you know, actually merge it merge the pull request into the project branch so that we can, you know, have some stable foundation that you can then build on top of and future changes. I'm not going to do anything with the draft we are like I'm going to make other PR and smaller PR, so that the review process gets easier. And I'll go and I'm going to close the PR the draft we are that I made after the things are migrated and stable. I mean once, once they once these pull requests that target the project branch are merged into the project branch, you'll be able to file additional pull requests that build on top of them. And, you know, we, you know, that way you can kind of work at your own pace implementing pieces of this one at a time and filing your requests for review that are a lot more focused. That was the whole idea of having a project branch in the first place. And, you know, we can look at the overall state of progress by comparing the project branch to the main branch. And the idea is eventually, there's going to be no more to do comments left. You know, right now there's, there's at least the to do comment that I added which is, you know, re enable the tests. So that's, that's an example of a to do comment that we wouldn't. be able to merge this project branch back into the main branch as long as that to do comment was unaddressed. Simply because we would never accept to have the main branch not do any testing. So it's the same, you know, the same idea all of these to do comments. It's okay to put them into the project branch, because this is a whole work in process, but we know that we're done with the project and ready to integrate the project into the main branch once there's no more to do comments left. Understood. So, no, in the good luck connection like, as you said, I am completely keeping the auditing good luck planning, as it is no changes at all, like I'm not even changing the name, like they're the same. The connection thing is not having much changes now. Now the main meat lies in the implementation. Now what I did was in the auto written get left client builder. I built the GitLab API instance directly in the build client by setting the ignore certificate and whatnot. And this, this will be returning me directly like when the user is going to select the auto detect ID. And the ordinal is also kept as it is and it's, and it's extending the GitLab client builder. Again, the version three and the version four also is doing the same thing it's also extending the GitLab client builder. And it is returning the client. I'll, I'll show you, like, I'll debug it also life, like, so that we don't have any doubt related to this. And regarding the GitLab client builder, it does not really have much change, like I was talking about this ordinal thing, like, you see, the, it's comparing like it cannot be zero because it's either zero either one or either two. So it will never be zero. When it's never, when it's never zero, then it's always returning me the difference. And I don't think it's useful at all. Like it's never comparing the IDs at all. It's only returning me the difference. Yeah, it's, it's, it's, I need to drink my cup of coffee before I can reason about Java ordinal calculations, because I always have to look up what the, you know, what the positive and negative numbers mean and that API but I'll check it over and see, see if I agree with your evaluation of it. Like, rest of this looks really good though what you just showed me in the last few files was looking really good. Like you, you can see here it's already sorting the builders according to the IDs that it's just comparing. So, I don't know. Let's see, maybe I'm missing something. I've got a question though is like, can we, can we change it from not zero to greater than one. Like, can you agree? Greater than or equal to one. So for not not zero, that's from the condition. It's actually the same like if the difference. It's like, it's kind of like, but do you think you want greater than one or equal to one. Okay. I don't think so. So, isn't the comp, as Basel said, isn't the compare to interface positives and negatives that not equals zero matters a bunch because it's, it's saying are they exactly the same is ordinal exactly equal to other dot ordinal. If you right click on compare to and go to go to the API docs for it if you click on right click on compare to and then select. I don't know if it's right click. Yeah, there you go. If you go, if you go back to that context menu and good and say go to super implementation. I think. Yeah. Yeah, there's a Java doc for it and few scroll up and read this. Yeah it's like, every time I look at this I have to read this Java doc again because it's really not very intuitive. It's like a one mathematics you know like this is not I was actually, I was actually talking to one of the Google developers on Twitter and he was complaining about this. And this is someone who's like written the guava library, he was complaining about this interface being, you know, clumsy to end to, you know, to remember which positives and negatives mean which mean which semantics so this is definitely something that a lot of people get confused about. It's normal but like, let me clear it all at once like what this method is trying to do is it gets all the client builders and tries to see if the ID is like version three or version four equals to the client that it that it has seen, and it is the provider, and we simply use the provider. So, it's just, I don't know where this thing is even used this ordinal thing is. It's not used anywhere. So, like, we'll, we'll see it afterwards. Other things, these are the simple things like I can answer this, I can answer this question with certainty in a day or two once I have some more time to read the, I don't think I can read it quickly enough in this meeting to come to any valuable conclusion. I was just explaining you my thoughts because I don't want you, I don't want you to be confused afterwards. That's why I was doing that. No, no. Yeah, we are using the client builder to build the client right to build the get library instant. That's the only place I was using it though. I did a quick check. Like, if you find something that you can ping me on the get a channel. Rest easy get live. No, we are, we are actually removing that was existing was in existing. Yeah, but we won't use cases for it. I don't think it is used in here. Like, I read it all and there's the proxy thing. 8080184 if you go back to the current main branch. Is it. Yeah, it's there. But it's like, no, no, no, you're actually not getting right. Like he, he used the ordinal thing, but it's still not be used in the implementation like it's good. It's getting into it's super and it's super will be get left client builder, and they get left client builder but get the ordinal but it but it will not do anything with the ordinal thing. That's what I was showing you, it's not doing anything with it. Yeah, but it seems like the intention was to use it in that compare to method which is used when sorting the builders. So, but it seems like your, your observation is that is that the compare to is not implemented in a way that uses the ordinal correctly. Yeah, that's something that I have to verify. Okay, okay, so let's move on to other things like the time is getting over. Yeah, I discussed it all. Attribute points. Yeah, so the get a merge request things like let me show you the, the mud the get a merge request that I removed from the cost data is here like it was used specifically. I added the code for it in the classes that use it specifically so that we don't require that at that point of time. And related to other changes. I don't think so there are any other major changes that I did. All seems to be working fine for me at least like I had one other blocker, but anything else that you would like to see like I don't think so I have anything else. Everything else seems to be working fine for me. No, this looks good so far. Have you, are you testing. Sorry, are you testing this against a Docker image of get lab locally. So how you're making these connections image. Nice. And harsh. I had the, yeah. No, go ahead. I'll, when you're when you finish, I'll, I've got another question about your testing also. Yeah, please go ahead. Yeah, so I go ahead. No, no, please. I'll let you finish you. I am going to distract you if I if I go along the path I was going so you finish and then I'll like I do interactive testing using the Docker images first, and then the production thing like the actual get lab server like for faster testing I use the get lab server. And if some if I have to test the version three or something then I'll use the Docker instance as well like, I think the version for was implemented from get lab version 13 or something, and I'm not sure yet. But yeah. Yeah, go ahead Mark. What's the problem. So, one of the things that I'll want to be able to do is do similar kinds of testing to what you're doing. Do you have Jenkins job definitions or pipeline definitions that you're using to make your testing a little easier. And if you don't do you objective I create those kind of things and put them someplace where I can share them. I, I find that no problem I, I didn't really think about this. So it could be a really great idea actually. Yeah, well I think you're probably just starting with clicking on that test connection button on the settings page, I would imagine because that's the most basic test. Is that is that we have that is that we started with her. Yeah, it almost always works like whenever I make it migrate things start working quite well, like, I'm not that bad at migration actually it works. Nice. So that's, that's a good first step. But yeah like what Marcus, as Mark was saying, once, once we have a connection, the testing shifts from, you know, being able to connect at all to making sure that each of these end points was migrated correctly with the correct semantics and the, the best way to test those is, is with an actual Jenkins job, because, you know, the unit tests will cover a lot of it. In a, in a sort of mock environment with the server returning the pre packaged data. But there's really not no substitute for for end to end testing, especially of the very common, common end points, you know things like, you know, triggering a build when when the floor quest gets updated these are like really common use cases that we would want to test using real Jenkins job, you know pipeline job or a freestyle job for example. I, I, I have tested it using the freestyle job, like the plugin that I'm using, I already migrated it, and the version three and version four both are there. So I'm showing you to like debugging it right now. And because I faced one of the blockers related to the, what was that called socket connection time out I guess. Yeah, like, so I'll be showing you. Yeah, so I selected the auto detection, I guess, yeah, that's why it is building the client using the auto detect ID. So I'll just step over all these things like this just to show that it's working. It's not like something problematic it's working like it just returns the client successfully and as the client was not already the client ID was not already present in the client case. So it just puts it in and returns the client client ID, it returns the connection map. Yeah, yeah, yeah, yeah, that's where the problem is, like it gets the client for me. That's perfect point. Yeah, this is looking great. I mean, this is looking, you know, basically just what I was hoping for from after the last review that I did so I think you've done a great job. No, it's, it's not that great actually like, let me show you what's the problem here. Now, when I come here, here comes the real problem. It's going to give me a GitLab API exception for some reason which I'm not able to understand, like, I should not get it. Just wait a minute. No, this is because attaching the debugger makes everything run more slowly. Right. Yeah, so yeah, it just hit there and it said me socket connection timeout with the connection timeout, timed out, which I completely understand how can the connection timeout. I'm not able to find any fork that I did in my migration, like everything is working really fine like there is no problem if I stop the debugger and just do the normal things that any user would do everything will be fine. So the way the way that I would debug this is what what what your code is doing is making a an HTTP request to a server. And when you're getting this socket timeout exception. It means that, I mean, I'd have to look, can you show me the whole stack trace, but what I would assume. If you expand the E in the debugger, I should be able to see it. Yeah, there we go. Can you expand the back trace. Oh, is about middle of the block. Yeah, there you go. Actually, that's, that's not what it is. Okay, I can't read that because it's all maybe maybe stack trace. Yeah, stack trace. That's what it meant. Okay, so towards the bottom of that. Whoops. Nope, you were in the in that in that expansion that you're in harsh stack trace the third from the bottom. Try that one. Okay, I heard back trace. You know, you heard me correctly, I just was thinking if they're on now can you make that wider so that I can see the whole thing. Yeah, so yes, it's kind of what I was kind of what I was thinking, what's the, what's at the bottom of this. It should be or actually, no, that's the top of this. Okay, here we go. Yeah, this is kind of what I was thinking so it's just, it's just not able to, it's, it's, if you navigate to element zero, you should see what. Yeah, if you click on the navigate button you'll see what's what's where this is actually going on. This doesn't, there's a navigate link I think right at the top. At the very right hand side, like next to line 700 word, navigate. Yeah, that's a few if you see that you're actually you haven't highlighted but if you look at the highlighted line on the very right hand side of it there's a navigate button. No, no, the highlighted stack trace line with the butter. Navigate. See the line navigate down down down down down. Stop. Back up. Yeah, you're now over the word navigate. Yeah, so click on that. Yeah, if you click on that it'll take you. So, you know, so you can actually, you know, put break points in here and get, get more, more detailed but I don't need to see more I can already see what's what's happening so the point is, you know, with the socket time out is that now the the GitLab API attempted to make an outbound TCP connection to the address that you gave it, and it couldn't connect and you would be able to you'd be able to reproduce that outside of Java by simply using the curl command and giving it the same, you know, the same arguments the same URL and, you know, header post and, you know, query parameters and body etc. You should be able to reproduce them with with curl, even outside of Java. This is nothing to do with with Java. This is just a matter of not being able to make an H make not only not not not only not being able to make an HTTP connection but being able to make any TCP connection at all to that address. So, you know that whatever address you gave it. You know, whether it's a local IP address or, you know, or maybe it's like a Docker hostname that that you have or like a virtual machine. I don't know exactly how you have your GitLab server running on your, on your machine. It seems like for whatever reason. You can't communicate with it from from from the client. So, you know, just because you can create that GitLab API object that doesn't mean that it actually talked to the server. And that's, and that's actually what the sort of negotiation thing does is that it actually does a really simple request from the server I think it's like get current user like who am I basically. And by doing and so I don't think you're doing that yet, because if you did that it would give you the same error messages you're seeing here. For whatever reason, the, you know, the IP address that you've entered or the hostname that you've entered is not routable from this environment that you're running into the GitLab server, whatever environment that's running in, you know, whether that's a Docker image or virtual machine or otherwise. So, and like I said, you can debug that pretty easily. You don't, you don't even need to debug it inside of Java. You can debug it just by writing a curl command that uses the same address and it should fail with the same time out. And then once you fix the problem, either by setting the hostname correctly or, you know, fixing your, your networking or whatever, then, you know, then it should start working after that. So, so Basel in terms of the debugging technique then if harsh stops the debugger just before it makes the request, he can inspect the content of the request in the debugger and prep the arguments to curl to match that request. Exactly. Okay, I was thinking. Yeah, you might find, you might find that it's something as simple as, you know, is getting the, the, you know, the, the hostname wrong or the IP address wrong or something. Are you, are you trying to connect to, where's the server that you're testing against is this the, the, the GitLab.com hosted server. Yeah, I'm using that currently I'm using that. So if I'm not using. If I'm not using that to that then, then, then it, we would assume that the hosted GitLab service is deployed correctly. So you can't connect to it. The most likely causes are, they are using the wrong address for some reason, or that I think so. But if you pointed that out, I think so because I'm not sure about the URL that I'm using, like it is possible that I, I have to use the base URL, like the host URL instead of the URL that I'm using currently. So, I have to check that, because it, I never really got into this trouble before like, when I, when I remove the version three then I use the base URL instead of the URL that the author has given but but I created the base URL myself without using the URL that is specified in the plugins code base by the author himself so maybe I can do that and it will start working again, but I just wanted to show you like the whole debugging process because like if something could be pointed out like I was doing something wrong or something like that, so that's why I did that. And the reason that it took, you know, 10 seconds or whatever to, to fail is, is that, you know, a TCP connection doesn't fail right away, you know, it, it sends, it sends the text and it, the, the TCP stack will wait, you know, I think it's like 10 seconds or something like that before it times out the same, the same way that if you went to a website in your browser like you know, I don't exist.com or something, it would take it would take about 10 seconds for that to fail as well. So, that's a handshake. Okay. Yep. I was going to show you that it was, was like, why is it so slow again, like faster. Yeah, well, part of it at least my experience has been when I'm screen sharing my computer is just generally slower. Right, there's a, there's a real price to be paid, there's a penalty to be paid when for sharing my screen. It should start on. Yeah, that's, that's what I'm like I'm not seeing any failure while I'm doing interactive testing at least with version four. Anything. It should give me a 500. What am I saying, it should not give me a 500. So, so harsh, this site that you're on right now. That's get lab.com right and it's trying to, it's trying to send data to your computer, I would have assumed your computer is blocked by a, what do you call it a NAT device and network attached a network translation device or something that would not allow get lab.com to reach to you but you've got n grok or something else configured to allow it in. Yeah, I, I did that like. Okay, it's in the middle actually. That's okay you don't need to show it to me your answer is great it says okay the the obvious things you've already handled. This is here. Yeah. Yeah, this is it. Okay, thanks. Otherwise it would not have been possible. Okay, I'm getting some creepy errors. Why do I get some creepy errors that I'm doing some live demo. That's bad. It's all right, you don't need to, you don't need to do a live demo to satisfy me. No, nor me nor any of us that's that's no problem. It was working something happening between like I'll check what happened. This is kind of crazy. But yeah, that's fine. Anything else that I needed to discuss like I discussed about the blocker regarding the docs I already already conveyed to Chris. And yeah everything I discussed. I don't think so I need anything else. I'll work upon it and maybe I'll make the PR tomorrow, like I'll try. I have some like, I'll have to test the responses. I don't know where it is. Yeah those the errors that I'm getting regarding the test is because of the responses that have not updated. So I'll have to do it. Otherwise, things are quite great. Like, I don't think so I have any problems. So anything that you would like to discuss more. Now this looks good so far. Yeah, nothing else for me. Meeting went wild. Like, it's already 1015. So, I think it's time to stop. All right, well let's I'll stop the recording then and we'll, it'll be uploaded in probably an hour or two. Thank you.