Magento Triage, 21 Jul 2020

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: Community Contribution Triage 07/21/2020 (Dev.Experience)

Description

Triaged issues:
https://github.com/magento/magento2/issues/28693
https://github.com/magento/magento2/issues/28463
https://github.com/magento/magento2/issues/28186
https://github.com/magento/magento2/issues/27397
https://github.com/magento/magento2/issues/27899

A

Hi valerie.

B

Hello, so hello, everyone, we started our regular public dev experience tickets triage. So I prepared some lists for today and.

A

Yeah, let me show you my.

A

A

A

Okay, this ticket about ipv6 uh protocol is supported to use 45 characters, and this is the main problem why we have the.

A

Issues and that's uh you know when we try to save a sales order and you and into two tables, because you know schema because declarations that russia's c2 and it's supported for ip is for addresses. But for ip66.

B

uh Yes, it looks like common issue, so could you scroll do we have some comment.

A

We have the fix for weight.

B

Could you open original issue x so what is destination.

B

So it's 2.4 develop so looks like we already have fixed. So yes, it's back because for ip version 6 we need more length, small and fields. So, let's, let's link pr, maybe it's already linked, so it should not be ready for development. Like should be fixed with pr which swizzy provided in comments.

A

The request is already linked.

B

Great, so maybe we can keep some comment like should be fixed in.

B

Could you open uh show comment from cesium? Maybe he already writes this fix it, but not much yet should be fixed.

B

Okay, we can say yes, we're waiting for margin of.

A

Yeah but we should set previously.

B

Let's keep p3 s3 uh because.

B

It's not some blocker. According to our policy, it's p3 s3 and I am not sure that it's so important and so usable feature like uh remote jp with ap version 6, especially if you already have fix so I believe it will be delivered and fix it since the near future, so yeah. I.

A

B

Know do we need to assign these tickets on on somebody like to avoiding that somebody can assign this ticket and start wars.

A

I think it's not pretty important, but here we can assign eagles reserve to this issue because he's there.

B

Maybe we need to provide description ticket assign it to your swizzy, because he always.

A

Says we are awesome, I think it's over here. We already have few links.

B

A

Title in the description and everywhere, okay.

A

Next uh swagger customer reboots is streaming, but not all at the boots at all screen, so contributor try to pass a string one two three: this is a write, instant category, this as a value by the our api, but he received the error is that the type should be array of the integer values, but he passed.

A

B

Additional commands, like information.

B

uh It's not uh-huh, it's really. Aha! Yes, it was rest call.

B

So uh looks like we need to check swagger because, according to error message, we need to put array of interval, but not string.

B

So how how it's work now, but maybe we have mistaken smoker. It's it's a little bit uh strange, because swagger is after generated based on declaration of our epa.

A

But I api here for value, as I know, for the declaration of mixer type, maybe.

A

A

Because I already saw some issues when we have the mixed type and swagger shows some wood for us, because swagger don't know anything about the mixer.

B

uh So looks like it's not bug, we can recommend to try to put array of intervalo instead of string and close. This.

A

We need to use string, but uh in real work we need to use a array of things, integer values, it's about conversion.

B

Maybe make sense to create separate bug like you need to adopt swagger, to understand and mix it well, as because I'm sure that, first of all that it will be easy task because mix it it's. uh We cannot predict what it suspects that when we talk uh that value is mixed.

A

Yeah, but currently we don't discuss how hard will be fixed. The issue trying to discuss uh is it is important or not. In this perspective, this issue is valid because us contributed provided mentioned. The swagger shows that value should be a screening. So we have this bug and it's really back and in general contributors say that parkinson's vlogger, not in our implementation and yeah. We should decide about exactly this, but what was this.

B

Sorry, maybe I missed it according to this description, uh it's mentioned that is bach, it's vlogger, or is it back in our product api? So if it is back in slogan, yes, we can keep it if it's.

A

First of all, the contributor provides information about that.

A

He tried to go uh to go through our documentation, so the squad documentation and pass stream as well so.

B

That's I told that is not bach. According to this uh step, to reproducing you should you should not put a string as well so expected result it's you should not. It's.

A

A really tricky question because our documentation said currently say it saying that we should pass dream. Yes,.

B

Documentation, but not with these steps.

A

It doesn't matter our documentation is, should go along with our implementation and if our documentation says something that is, it means that we should follow this uh steps. Let's.

B

A

B

Somebody pick up this ticket so, for example, I would like to fix this issue. It's confirmed that we have of some priority uh what they see. I need to send string expectations that it should work, so this guy, who will pick up this ticket, will try to fix issue that you should work with string according to description.

B

So maybe we can keep this issue but need to change description and provide that you should should not fix this case. You should fix swagger, but not like it's described in topic.

A

Yeah, I agree that we should change the description of this.

A

Instead of our implementation.

B

So if you talk about fixing of swagger for me, it's like severity, two, because I'm not sure about priority p2 or p3, but severity is two because a lot of guys uh opens vlogger and uses like source of trust, and you should spend a lot of time for investigation. What is the issue? Do I have issue with my code?

B

Do I have issue with magento code, or maybe it's issue with documentation, so it's a lot of developer will read this documentation and I believe that we have a lot of another very familiar cases when uh type is mixed, but about priority, I'm not sure what it is know it's picture or pp3. Maybe japa can't help us.

A

My point of view is cp3 because yeah it's important, but we have the really clear description where we say that uh police pass array of integer and.

B

A

B

A

To try first to pass string and he will see the clear error message after that he will try to use things and.

B

A

Will be okay, so I think we can say.

B

Yes, because we have a good error message, I agree.

A

Okay- and I will mark this issue that I need to change the description.

A

Okay: let's go to the next one.

A

A running bin, magenta setup upgrade will enable layout block, html and full page caches the step to produce, let's disable cache by our command for blocks creation layout after that run, upgrading, and we will see that cache will be enabled.

B

B

So I see that maybe most issue related is mostly to development or some staging server, because if you talk about production.

B

You should have enabled cache, so there are should not be issue if I upgrade on production server after that, I still have enabled caching says it's preference table so for production. It should not be big issue, but it could be some issue for debugging if I am working on staging, so I disable all of caches for debugging make a braid after that, I need to wrap date, disable the sketches.

B

So first of all we have workaround. Yes, you need manually to disable all of caches. What do you need? Also for me, it's not related to production. I don't know. Maybe somebody use production with disabled cache, but it's a little bit strange situation. So that's why.

A

B

A

It has a lot of lights, but I'm also don't understand the reason of it because yeah first of all, we have the workaround and yeah. If we do upgrade, we can do after that disable cache again it's. I don't think this is a big problem.

B

So um my initial opinion was like before as severity 4, but we have some likes, so that's mean that issue is really annoying for our developers. So, let's, let's uh increase priority like p3 s3. So if there are was not any light, so my opinion it's before, but only that we have some likes on this issue.

A

Okay, let's set pp3, maybe I don't know I will leave the comment with the recording to this ticket and maybe someone who said a like for this issue. We will just have additional information, maybe.

B

Basically, could you show comment? Maybe we have explanation uh some real situation? Why is it important.

B

It just asked me reserved here some proposal mode.

B

So, for me, it's yes! If, if you had disable a cache, this cache should be disabled after updating the same with enabling. So for me it's it's. A logical, predictable.

B

So do we have? There are some final resolution, how it should work.

B

A

B

B

We have some questions, but maybe we need to provide answer like yes, where is? It is bar and desire state? Its cash should be in the same state as it was before. Upgrading.

A

As it was set manually,.

B

Maybe we have some restriction from magento site. That's why it's implemented in this way, it's hard to know without submerging in code. But if we look on these issues, it's more logical to have the same state.

A

Currently, we have the request for this issue and if it fix the problem- and also we can see the patch for that, so I don't think that it's some magenta.

B

Restriction, it's a little bit hard for testing because you should test upgrade and you should test that upgrade was uh without any errors.

B

So for me it's it's a little bit complicated issue just because there are. You should test this locally, because our deployer doesn't support testing of upgrades.

B

So there should be good manual testing.

A

Okay: let's go to the next tickets.

B

I don't know, maybe we can uh make some notches for sergio, but like pay attention, there should be more jeep testing that we have another issue.

C

Guys, please leave it, it's a comment, because it's not only for me personally who will work on testing all issues, but.

A

C

Help you know each person will.

B

Work so the issue that is required or should be tested manually should be tested.

B

Upgrade now uh we are working on some tools which will help us to work with this issue, but functionality is under development.

A

A

Duplicate name, while adding indexes and constraints in db, schema.

B

Key is entity id just second primary key is entity id and initial state. We have one more index with identifier reference id okay,.

A

Primary is important in this situation and second, we that was.

B

A

Additional constraint.

B

Unique, I'm sorry could you show on one screen uh first block? ah Yes,.

A

That's the same just edit one constraint.

B

Just let's check yes looks like it's the same so and what is uh what is error? What is proposal.

A

B

Are created and we have duplicate.

B

So basically, magenta is enabled to generate unique name for indexing, the constraint okay, and we have some issue reference in in this tickets. Could you also open cheek issue.

A

This issue related to db's list.

B

A

B

It looks like a big proposal. According to our db scheme,.

B

Okay, I think there I need more time for understanding what is proposal of this issue. So, let's back to our.

A

Yeah, I think we need to jump more deeply to this issue, because it.

B

Looks like we can uh for me. It's main concern that we try to create index and constraint on the same field. So we are constrained by constraint. You can't create primary key or unique, but it's it's index you. If you use constraint, you already create index like primary. If you use constraint, you already have index like unique, but you, uh but in this case we try to create one more index on the same field. As a result, you will have uh two index in the table.

B

One of them will be uh just simple index b3 index and second, it will be unix index yes looks like it's bug. Maybe magenta does not know, doesn't support after generation for constrain and for index. But if you talk about real life for me, it's hard to imagine some situation.

B

When you need index and unique ended on the same field. It looks like you try.

A

B

Do something bad in your.

A

A

In general engines that we use in this situation support this behavior, so we also as a client should support this behavior. Yes,.

B

Yes, it's absolutely it's clear. It's issues that we cannot provide.

A

Maybe maybe I'm not maybe I'm wrong, but in this declaration maybe contributor want to upgrade this hema. So what happens here? It doesn't matter. The previously index was not unique, partly he want to make this index unique, but he can't override this.

B

uh But now, if you would like to upgrade, if another mistake need to check our documentation, you should remove the first declaration, because in final declaration you have declared two fields. If you would like to overwrite like, I would like to have uh some another index with the same name. For example, it was a simple index b3 index and I would like to introduce unique index, but with the same name. In this case you should remove index declaration.

B

Honestly, I don't remember syntaxes, but you should provide all index disabled true, it should be attribute disabled or you need just remove this field. But in this declaration you.

A

B

Yes, you should remove this from declaration.

A

How you can remove, if you edit it from if you do it from your model, you want to update the schemas are declared in another model, is xml will be measured?

A

So let's look to the schema to this declaration like.

A

Declaration in all models for this table.

B

But okay, if even it will be merged it. First of all, it leads to strange situation when you will have uh two index on one field: one unique unique index and another uh b3 index. Yes, I agree that it's back that we need to have uh after.

B

Yes, absolutely sure that, yes, because it's a pretty common situation, so just second, for example,.

B

B

uh You should provide it's from documentation. You should redeclare it with the disable attribute set to true.

B

So for me I agree that is bug. We have issues after key generation, but for me it's p4 severity 4, because, honestly, I cannot imagine case from real life when you need, for example, primary or unique at the index on the same column, because it's already indexed.

B

Moreover, if you have the situation, if you have, the situation, looks like you have some issue with architecture for your db of your table.

A

Yeah, I think, in this case contributor didn't know about the disabled functionality of previous old index and that's the reason why he tried just to add additional index in this case.

B

We can clarify it, but if I would describe it it's before.

B

A

Okay, we have just one minutes to change. Let's.

B

Check one more final.

A

Maybe let's go to the next meeting: okay, so.

B

A

Okay, stop sharing! Thank you.

B

You forgot to say thank you. The second fall for all of our visitors.

A

Thank you guys for coming you're. Welcome guys my.

A