GitLab It's time to Go!, 17 Aug 2022

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: It's time to Go! Episode 2

Description

Informal chat about the Go programming language.

In this episode we are addressing https://gitlab.com/gitlab-org/cluster-integration/gitlab-agent/-/issues/86.

A

I uh we are here at the it's time to go, call or talk about google and we forgot to record it unfortunately from the beginning, but we remember now and we will go through what we've discussed today so.

A

We're talking, we were looking to issues on the gitlab agents.

A

Ripple that are good for new contributors, and we found this one which has to do.

A

To converting enum to strings because we believe that uh there is a problem with phenoms where we can't uh it's, we can't use the same names. I think that's that's correct. Let's say you can't use the same enough names without having them as unique. Is that a good description, or would you describe it differently.

B

So all all enums in a single package have to be named differently, like the values for the nums have to be named differently, because the values share the namespace. They don't belong to a particular in enum. Basically, and that's just annoying, you can't have two enums with the same value. If you rephrase it that's the thing, so we decided a long time ago that we want to change this and um to a string and just use validation on top of that string.

B

Instead of in um doing the validation for us, and we can actually do a better job with validation.

A

So we had this enum right here that we removed and everywhere that we were calling it. We converted to string like that, make it an optional string and we added some protocol forgot how those are called: okay, validation or validation. But there's uh each one of those are what um this.

A

Right so you're just adding validation to match what you know had before using this in predicate, I don't know, what's going on.

B

And another interesting property here, property of this change is that before we were accepting.

B

A string with an empty field with an empty value, and that would mean the default value which is gcp and now we become more strict and an empty string will not be allowed by validation.

B

uh If you want to default, I meet the the value I need the the field or in your yaml, so do not specify network. If you want default, you don't specify an empty string if you want default and we can disambiguate or distinguish between these two by having the optional there. So we track if the field is present or not it's a new feature that was added a year ago, so to proto3 it wasn't available.

C

A

And also like because the nums were like, uh I think they default naturally to zero because they are integers. I think right and now we have to also apply the default, because, uh like um said, we cannot have like that empty string, design people, so we need to also implement like uh the defaults of what this, if the user does not pass, it then once they before.

A

So this is what we are doing here. Also like.

A

About a good example: okay, so so yeah, so we have like a defaulting protocol defaulting file.

A

And well, let's just open this file.

A

And here we implement uh functions to basically apply the defaults and we didn't have um a function that would apply the default to a string, pointer or a pointer to a string, and this is basically what we got from protocol buff from protobuf after we regenerated the protocol, the protobuf.

A

A

A

To this one that got generated, you can look for a network and see that it has been written a pointer to a string. So that's why you created that.

A

That string before or yeah string pointer, which will apply, apply the default, and this this step will be applied on the when, where we use that network thingy, which is basically on the cast app.

A

And this is on agent's default agent server, which is here so you were just about like to write that here we implemented the method. Now you can pass.

B

And I suggest you currently all the fields uh the defaulting happens in the order of the fields, so please move it above, so that we maintain the order.

B

um I think it should be above the address if I'm not wrong, if you click here, if you click at the address.

B

Here, command click network.

A

Is above it, so, let's.

B

Put the network deporting above about this thing.

B

A

And this is miss, then listen.

A

And we need to set the default and also just.

B

A

The full agent to listen network.

A

We can say tcp.

A

And then it is.

A

We need tests for this one.

B

No, uh what we need is we need.

B

To do this in a few a few more places everywhere, the network was used um where your nam was used. Sorry.

A

It might have just been.

B

So if you look open the protofile, you see where it was used.

A

A

A

Listen kubernetes api, but this will map to the generated file.

A

Which is this one.

A

Maybe from here.

B

You can search for the where we are defaulting the address, maybe or.

A

Configure that, because this is this,.

B

This is for the kubernetes.

A

There's also that before we were converting the thing to a string, and now it's.

A

Pointer to string.

B

So, for other places where this is used, they are probably in modules.

B

You can find this by looking at the address defaulting search for your the address and see where it is defaulted and then, at the same place you need to default. um No just go to that. Struct like here address on this on this line. No 27.

B

And search for address way where it is used.

A

Something here.

B

A

Here, for example,.

B

We need to define the.

A

B

Default network for that for the observability server.

A

We need to read the defined.

B

Yeah this is the observability server. Okay. Now uh I think this was a separate type for this. The observability listen cf, but then we also need to look at what other addresses are there to find the matching networks.

A

And this I can find the best web here.

A

Red server, config, probably.

B

A

B

This is um this, doesn't have, does it no? No, this is not the one, so maybe we can start from the proto go from the proto file and just look at where the fields are that we.

A

Changed so there is a giant.

B

Yeah, I think this should be in the kubernetes api module.

A

B

No server and defaulting.

A

C

A

B

And then the module should probably be updated to.

B

The reference, the pointer there is probably an error somewhere in in module or in the proxy.

B

Only if you open it, it doesn't try to evaluate all the files.

B

Cool and now and then there's probably more what else did we change.

B

A

This is the agent's accent.

B

This needs to be changed to be optional, but later.

A

Listen api conflict. Have we done this.

B

No, I don't think so. ah Wait api, yeah! No, no.

C

A

Where does this get used.

A

um Private data.

B

C

B

Be in the defaulting in the main d40.

A

C

B

In tools now in the in the class default file,.

A

But this is done.

B

That should be line 45.

B

And then line for gl 48 as well for.

A

Api and private api yeah.

C

A

I didn't remember any of this.

C

A

C

A

B

C

A

C

A

We still have more.

B

B

I need just count how many, how many have you changed.

A

B

Not one why one the country.

C

B

Multiple, I could have continues a lot yeah.

A

B

Let's look at the profile and see what what we, what we've done there.

A

Yeah, this was easy listing coffee.

A

This is done, observability we've done this is done, and this is done right. Yes,.

B

ah There should be one more which we have changed to the private api.

B

Is that all the diff.

A

Yeah for the protofile- yes interesting.

A

Can you guess the private api use this one? No.

B

ah Yeah, maybe there's several that use the same. Listen api, uh struct, yes, api.

C

B

Exactly okay and have we you? We have changed both cool.

A

If you haven't.

C

Changed the ci.

A

Will tell us, I guess, yeah well,.

B

C

I think, because.

B

You need to run the tests, uh something might actually fail. Also, we have the uh that.

B

The test that generates the example configuration file for cast and it will probably almost most likely fail. Now I yeah I can take example test.

B

B

One so it takes this half empty file and applies defaults to it and then compares.

C

B

With the yaml file that we have in the same directory, and if they are.

A

Not the same, then it.

B

B

Yeah, it's just meant to keep the example up to date that yama yeah, who do you tell us that network is missing.

B

Only in one place.

A

Should be empty, but was now that okay, this.

B

Is the error? The div should be empty yeah, but there was.

A

Network strength is for privately decent private dpi yeah.

B

It's probably below yeah.

A

And listening to both address network, which is tcp.

A

And you can just rerun.

A

See if it faces.

C

B

Cool, well, you can also try running cast locally.

B

Yeah to see if it starts.

B

And you can also edit the configuration file that you supply and remove all the net network uh yeah. It looks like it's. Okay, it started and it means all the network strings are provided.

A

B

Yeah, try removing all the network strings from the config to see if it defaults them correctly.

B

Do you know where the config is.

A

It's in the gitlab gdk and then there's youth, lab and coffee. It's this one.

A

A

Comment them out.

A

B

Okay, it works.

A

Okay, it's running just to confirm the cast viewed so pointing to.

A

A

A

Cool, let's commence and push.

A

And it's actually converts.

B

Now you are committing the idea, folder.

A

The patch now we can still not connect it.

B

That's why I use intellij or golens to commit things.

B

It's more visual, so, but here.

C

A

B

Visual, you, you look at what you do, what you are committing here.

B

B

A

I never know how to.

B

Explain this same same.

B

Have enough value name in different amounts.

B

Something like that.

C

A

ah I'll change the remote uh url with the ssh okay.

B

Okay, I think that was a good session.

B

Thanks for doing this work, it's good to clean things up and simplify things. If you have a little bit more time later, not right now and we have, we also have them.

B

I think radius uses the empty string as the default and then we can change it to optional to disallow the empty string. Basically, if you, if you look for that string, validation and yeah in the profile, uh just look for an empty string. Basically, two double quotes uh yeah. This needs to be changed to optional.

B

A

B

Defaulting will also change to pointer, but yeah. Let's do it separately in a separate mr.

A

Interesting that this was already done before, or network.

B

A

Because this will be done later.

B

I think we did it later. That's why it's in the new way, but optional was not available when we were doing this, so now is a good time to convert it to optional.

B

There are more things that we need to convert to optional. Just you need to find time.

A

Okay, go thanks.

A