[neutron] Please hold off on merging patches
Hi Neutrinos, It was noticed today by Mohammed Naser that the neutron gate was only running the docs and pep8 jobs. We were able to track it down to the RE2 patches that were recently merged [0]. Until the reverts land please don't +W anything since it could merge untested and would just need to be reverted. Other projects might have similar issues if they updated code using the 'negate' directive for regex recently as we found it doesn't actually work with RE2 when using negative lookahead [1]. Thanks, -Brian [0] https://review.opendev.org/q/topic:"files-negate" [1] https://github.com/google/re2/issues/156
On Tue, Jun 4, 2024, at 1:06 PM, Brian Haley wrote:
Hi Neutrinos,
It was noticed today by Mohammed Naser that the neutron gate was only running the docs and pep8 jobs. We were able to track it down to the RE2 patches that were recently merged [0]. Until the reverts land please don't +W anything since it could merge untested and would just need to be reverted.
Other projects might have similar issues if they updated code using the 'negate' directive for regex recently as we found it doesn't actually work with RE2 when using negative lookahead [1].
Note this is very misleading. It is true that re2 does not support negative lookahead. Zuul's implementation does a positive match then negates the result. This is why you have to say things like regex: foo negate: true. This works outside of typical regex syntax to negate the result. The problem here is not re2 or negative lookahead regexes themselves but instead a conversion to non equivalent regular expressions. If we look at an ironic example [2] the old working regex with native negative lookahead support said "all files in tools/ except for bandit.yml are an irrelevant file". The new converted regex said "all files but tools/bandit.yml are an irrelevant file". This meant only changes including updates to tools/bandit.yml would trigger the job. We do still need to drop the use of native negative lookaheads in zuul configs as re2 regexes will be the only valid regexes in future zuul releases. Quick reverts are a good initial step, but I don't want to have an impression this is something we can revert then leave as is.
Thanks,
-Brian
[0] https://review.opendev.org/q/topic:"files-negate" [1] https://github.com/google/re2/issues/156
[2] https://review.opendev.org/c/openstack/ironic/+/921319/1/zuul.d/ironic-jobs....
On Tue, Jun 4, 2024 at 4:52 PM Clark Boylan <cboylan@sapwetik.org> wrote:
On Tue, Jun 4, 2024, at 1:06 PM, Brian Haley wrote:
Hi Neutrinos,
It was noticed today by Mohammed Naser that the neutron gate was only running the docs and pep8 jobs. We were able to track it down to the RE2 patches that were recently merged [0]. Until the reverts land please don't +W anything since it could merge untested and would just need to be reverted.
Other projects might have similar issues if they updated code using the 'negate' directive for regex recently as we found it doesn't actually work with RE2 when using negative lookahead [1].
Note this is very misleading. It is true that re2 does not support negative lookahead. Zuul's implementation does a positive match then negates the result. This is why you have to say things like regex: foo negate: true. This works outside of typical regex syntax to negate the result.
I think neutron's problem was assuming that negate will be in-place replacement for the use case. Now I don't see how it could in this specific scenario (where irrelevant-files filters are ORed.)
The problem here is not re2 or negative lookahead regexes themselves but instead a conversion to non equivalent regular expressions. If we look at an ironic example [2] the old working regex with native negative lookahead support said "all files in tools/ except for bandit.yml are an irrelevant file". The new converted regex said "all files but tools/bandit.yml are an irrelevant file". This meant only changes including updates to tools/bandit.yml would trigger the job.
We do still need to drop the use of native negative lookaheads in zuul configs as re2 regexes will be the only valid regexes in future zuul releases. Quick reverts are a good initial step, but I don't want to have an impression this is something we can revert then leave as is.
We are landing definitions that will list all files to ignore explicitly: https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/921330 https://review.opendev.org/c/openstack/neutron/+/921309 I suspect this could be cleaned up to reuse a single variable listing files to ignore through */& syntax.
Thanks,
-Brian
[0] https://review.opendev.org/q/topic:"files-negate" [1] https://github.com/google/re2/issues/156
[2] https://review.opendev.org/c/openstack/ironic/+/921319/1/zuul.d/ironic-jobs....
---- On Tue, 04 Jun 2024 18:15:49 -0700 Ihar Hrachyshka wrote ---
On Tue, Jun 4, 2024 at 4:52 PM Clark Boylan cboylan@sapwetik.org> wrote: On Tue, Jun 4, 2024, at 1:06 PM, Brian Haley wrote:
Hi Neutrinos,
It was noticed today by Mohammed Naser that the neutron gate was only running the docs and pep8 jobs. We were able to track it down to the RE2 patches that were recently merged [0]. Until the reverts land please don't +W anything since it could merge untested and would just need to be reverted.
Other projects might have similar issues if they updated code using the 'negate' directive for regex recently as we found it doesn't actually work with RE2 when using negative lookahead [1].
Note this is very misleading. It is true that re2 does not support negative lookahead. Zuul's implementation does a positive match then negates the result. This is why you have to say things like regex: foo negate: true. This works outside of typical regex syntax to negate the result.
I think neutron's problem was assuming that negate will be in-place replacement for the use case. Now I don't see how it could in this specific scenario (where irrelevant-files filters are ORed.) The problem here is not re2 or negative lookahead regexes themselves but instead a conversion to non equivalent regular expressions. If we look at an ironic example [2] the old working regex with native negative lookahead support said "all files in tools/ except for bandit.yml are an irrelevant file". The new converted regex said "all files but tools/bandit.yml are an irrelevant file". This meant only changes including updates to tools/bandit.yml would trigger the job.
Yeah, and that is why "negate=true" was considered as direct replacement of "?^" negative regex for zuul config which make it wrong when used in 'irrelevant-files' or 'files'
We do still need to drop the use of native negative lookaheads in zuul configs as re2 regexes will be the only valid regexes in future zuul releases. Quick reverts are a good initial step, but I don't want to have an impression this is something we can revert then leave as is.
So what is the solution here? adding all files explicitly in either 'irrelevant-files' or 'files' does not look good as it can end up having a very lengthy file or need updates every new file is added in that directory. And we cannot add both 'irrelevant-files' or 'files' together. Also, another point is that if Zuul converts these warnings to errors then this will have another big impact on OpenStack where things were working in the past and ended up in error. Can we keep Zuul more backward compatible especially for the things already being in used so many places? -gmann
We are landing definitions that will list all files to ignore explicitly: https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/921330 https://review.opendev.org/c/openstack/neutron/+/921309
I suspect this could be cleaned up to reuse a single variable listing files to ignore through */& syntax. >
Thanks,
-Brian
[0] https://review.opendev.org/q/topic:"files-negate" [1] https://github.com/google/re2/issues/156
[2] https://review.opendev.org/c/openstack/ironic/+/921319/1/zuul.d/ironic-jobs....
On Wed, Jun 5, 2024, at 10:38 AM, Ghanshyam Mann wrote:
On Tue, Jun 4, 2024 at 4:52 PM Clark Boylan cboylan@sapwetik.org> wrote: On Tue, Jun 4, 2024, at 1:06 PM, Brian Haley wrote:
Hi Neutrinos,
It was noticed today by Mohammed Naser that the neutron gate was only running the docs and pep8 jobs. We were able to track it down to
patches that were recently merged [0]. Until the reverts land
---- On Tue, 04 Jun 2024 18:15:49 -0700 Ihar Hrachyshka wrote --- the RE2 please
don't +W anything since it could merge untested and would just need to be reverted.
Other projects might have similar issues if they updated code using the 'negate' directive for regex recently as we found it doesn't actually work with RE2 when using negative lookahead [1].
Note this is very misleading. It is true that re2 does not support negative lookahead. Zuul's implementation does a positive match then negates the result. This is why you have to say things like regex: foo negate: true. This works outside of typical regex syntax to negate the result.
I think neutron's problem was assuming that negate will be in-place replacement for the use case. Now I don't see how it could in this specific scenario (where irrelevant-files filters are ORed.) The problem here is not re2 or negative lookahead regexes themselves but instead a conversion to non equivalent regular expressions. If we look at an ironic example [2] the old working regex with native negative lookahead support said "all files in tools/ except for bandit.yml are an irrelevant file". The new converted regex said "all files but tools/bandit.yml are an irrelevant file". This meant only changes including updates to tools/bandit.yml would trigger the job.
Yeah, and that is why "negate=true" was considered as direct replacement of "?^" negative regex for zuul config which make it wrong when used in 'irrelevant-files' or 'files'
No, this isn't necessarily wrong for irrelevant-files or files. It depends on what you are trying to achieve. You need to write regex rules that match the files appropriately for your goals. That may end up using negate: true. This is why I said the previous statement is misleading. It made it sound like negate: true is fundamentally broken. It isn't; it does what it says on the tin. Unfortunately, that wasn't what you intended it to do.
We do still need to drop the use of native negative lookaheads in
zuul configs as re2 regexes will be the only valid regexes in future zuul releases. Quick reverts are a good initial step, but I don't want to have an impression this is something we can revert then leave as is.
So what is the solution here? adding all files explicitly in either 'irrelevant-files' or 'files' does not look good as it can end up having a very lengthy file or need updates every new file is added in that directory. And we cannot add both 'irrelevant-files' or 'files' together.
For Neutron it appears that the intention of those irrelevant files rules is to keep jobs from running when source code is not modified. I made a suggestion in IRC today that Neutron update their jobs to do something like: files: - ^.*\.py - zuul.d/project.yaml irrelevant-files: - ^neutron/tests/unit/.*$ - ^neutron/tests/fullstack/.*$ - other overmatch exclusion rules here This matches the intended behavior of "only run this job when source code is changed" much better than managing a massive amount of irrelevant files rules. In the ironic case they decided they could just run bandit jobs more often. I think this is also a valid approach as we often try to over optimize these rules and confuse ourselves (the current situation is an excellent example of this).
Also, another point is that if Zuul converts these warnings to errors then this will have another big impact on OpenStack where things were working in the past and ended up in error. Can we keep Zuul more backward compatible especially for the things already being in used so many places?
Zuul is intentionally taking this slow as we knew this would be impactful to openstack and others. The backward compatibility path is warning projects that this will become an error giving people plenty of time to convert their rules. That said eventually we do need to stop using the builtin regex library.
-gmann
We are landing definitions that will list all files to ignore
explicitly:
https://review.opendev.org/c/openstack/neutron/+/921309
I suspect this could be cleaned up to reuse a single variable
https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/921330 listing files to ignore through */& syntax. >
Thanks,
-Brian
[0] https://review.opendev.org/q/topic:"files-negate" [1] https://github.com/google/re2/issues/156
[2] https://review.opendev.org/c/openstack/ironic/+/921319/1/zuul.d/ironic-jobs....
On 6/5/24 3:05 PM, Clark Boylan wrote:
On Wed, Jun 5, 2024, at 10:38 AM, Ghanshyam Mann wrote:
On Tue, Jun 4, 2024 at 4:52 PM Clark Boylan cboylan@sapwetik.org> wrote: On Tue, Jun 4, 2024, at 1:06 PM, Brian Haley wrote:
Hi Neutrinos,
It was noticed today by Mohammed Naser that the neutron gate was only running the docs and pep8 jobs. We were able to track it down to
patches that were recently merged [0]. Until the reverts land
---- On Tue, 04 Jun 2024 18:15:49 -0700 Ihar Hrachyshka wrote --- the RE2 please
don't +W anything since it could merge untested and would just need to be reverted.
Other projects might have similar issues if they updated code using the 'negate' directive for regex recently as we found it doesn't actually work with RE2 when using negative lookahead [1].
Note this is very misleading. It is true that re2 does not support negative lookahead. Zuul's implementation does a positive match then negates the result. This is why you have to say things like regex: foo negate: true. This works outside of typical regex syntax to negate the result.
I think neutron's problem was assuming that negate will be in-place replacement for the use case. Now I don't see how it could in this specific scenario (where irrelevant-files filters are ORed.) The problem here is not re2 or negative lookahead regexes themselves but instead a conversion to non equivalent regular expressions. If we look at an ironic example [2] the old working regex with native negative lookahead support said "all files in tools/ except for bandit.yml are an irrelevant file". The new converted regex said "all files but tools/bandit.yml are an irrelevant file". This meant only changes including updates to tools/bandit.yml would trigger the job.
Yeah, and that is why "negate=true" was considered as direct replacement of "?^" negative regex for zuul config which make it wrong when used in 'irrelevant-files' or 'files'
No, this isn't necessarily wrong for irrelevant-files or files. It depends on what you are trying to achieve. You need to write regex rules that match the files appropriately for your goals. That may end up using negate: true. This is why I said the previous statement is misleading. It made it sound like negate: true is fundamentally broken. It isn't; it does what it says on the tin. Unfortunately, that wasn't what you intended it to do.
Understood, it was our misunderstanding of things that lead to the breakage, and I don't think we can get it to work correctly in the irrelevant-files section based on testing.
We do still need to drop the use of native negative lookaheads in zuul configs as re2 regexes will be the only valid regexes in future zuul releases. Quick reverts are a good initial step, but I don't want to have an impression this is something we can revert then leave as is.
So what is the solution here? adding all files explicitly in either 'irrelevant-files' or 'files' does not look good as it can end up having a very lengthy file or need updates every new file is added in that directory. And we cannot add both 'irrelevant-files' or 'files' together.
For Neutron it appears that the intention of those irrelevant files rules is to keep jobs from running when source code is not modified. I made a suggestion in IRC today that Neutron update their jobs to do something like:
files: - ^.*\.py - zuul.d/project.yaml irrelevant-files: - ^neutron/tests/unit/.*$ - ^neutron/tests/fullstack/.*$ - other overmatch exclusion rules here
This matches the intended behavior of "only run this job when source code is changed" much better than managing a massive amount of irrelevant files rules. In the ironic case they decided they could just run bandit jobs more often. I think this is also a valid approach as we often try to over optimize these rules and confuse ourselves (the current situation is an excellent example of this).
Thanks for the info, I did open a bug to track this as a better (read: shorter) way to accomplish what the old "?!(pattern)" expression was doing. -Brian [0] https://bugs.launchpad.net/neutron/+bug/2068523
Also, another point is that if Zuul converts these warnings to errors then this will have another big impact on OpenStack where things were working in the past and ended up in error. Can we keep Zuul more backward compatible especially for the things already being in used so many places?
Zuul is intentionally taking this slow as we knew this would be impactful to openstack and others. The backward compatibility path is warning projects that this will become an error giving people plenty of time to convert their rules. That said eventually we do need to stop using the builtin regex library.
-gmann
We are landing definitions that will list all files to ignore
explicitly:
https://review.opendev.org/c/openstack/neutron/+/921309
I suspect this could be cleaned up to reuse a single variable
https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/921330 listing files to ignore through */& syntax. >
Thanks,
-Brian
[0] https://review.opendev.org/q/topic:"files-negate" [1] https://github.com/google/re2/issues/156
[2] https://review.opendev.org/c/openstack/ironic/+/921319/1/zuul.d/ironic-jobs....
---- On Wed, 05 Jun 2024 12:05:45 -0700 Clark Boylan wrote ---
On Wed, Jun 5, 2024, at 10:38 AM, Ghanshyam Mann wrote:
On Tue, Jun 4, 2024 at 4:52 PM Clark Boylan cboylan@sapwetik.org> wrote: On Tue, Jun 4, 2024, at 1:06 PM, Brian Haley wrote:
Hi Neutrinos,
It was noticed today by Mohammed Naser that the neutron gate was only running the docs and pep8 jobs. We were able to track it down to
patches that were recently merged [0]. Until the reverts land
---- On Tue, 04 Jun 2024 18:15:49 -0700 Ihar Hrachyshka wrote --- the RE2 please
don't +W anything since it could merge untested and would just need to be reverted.
Other projects might have similar issues if they updated code using the 'negate' directive for regex recently as we found it doesn't actually work with RE2 when using negative lookahead [1].
Note this is very misleading. It is true that re2 does not support negative lookahead. Zuul's implementation does a positive match then negates the result. This is why you have to say things like regex: foo negate: true. This works outside of typical regex syntax to negate the result.
I think neutron's problem was assuming that negate will be in-place replacement for the use case. Now I don't see how it could in this specific scenario (where irrelevant-files filters are ORed.) The problem here is not re2 or negative lookahead regexes themselves but instead a conversion to non equivalent regular expressions. If we look at an ironic example [2] the old working regex with native negative lookahead support said "all files in tools/ except for bandit.yml are an irrelevant file". The new converted regex said "all files but tools/bandit.yml are an irrelevant file". This meant only changes including updates to tools/bandit.yml would trigger the job.
Yeah, and that is why "negate=true" was considered as direct replacement of "?^" negative regex for zuul config which make it wrong when used in 'irrelevant-files' or 'files'
No, this isn't necessarily wrong for irrelevant-files or files. It depends on what you are trying to achieve. You need to write regex rules that match the files appropriately for your goals. That may end up using negate: true. This is why I said the previous statement is misleading. It made it sound like negate: true is fundamentally broken. It isn't; it does what it says on the tin. Unfortunately, that wasn't what you intended it to do.
We do still need to drop the use of native negative lookaheads in
zuul configs as re2 regexes will be the only valid regexes in future zuul releases. Quick reverts are a good initial step, but I don't want to have an impression this is something we can revert then leave as is.
So what is the solution here? adding all files explicitly in either 'irrelevant-files' or 'files' does not look good as it can end up having a very lengthy file or need updates every new file is added in that directory. And we cannot add both 'irrelevant-files' or 'files' together.
For Neutron it appears that the intention of those irrelevant files rules is to keep jobs from running when source code is not modified. I made a suggestion in IRC today that Neutron update their jobs to do something like:
files: - ^.*\.py - zuul.d/project.yaml irrelevant-files: - ^neutron/tests/unit/.*$ - ^neutron/tests/fullstack/.*$ - other overmatch exclusion rules here
This matches the intended behavior of "only run this job when source code is changed" much better than managing a massive amount of irrelevant files rules. In the ironic case they decided they could just run bandit jobs more often. I think this is also a valid approach as we often try to over optimize these rules and confuse ourselves (the current situation is an excellent example of this).
Oh, I think I was told/or read somewhere that using both together 'files' and 'irrelevant-files' is unknown behaviour. It may or may not work as expected. If it is confirmed/tested behaviours from Zuul that we can mention both then this will help. In the above example, which one takes precedence ('files' or 'irrelevant-files') where there is overlap? What happens if any change in .py file under neutron/tests/unit/ ? 'files' tells run the job on any .py file and 'irrelevant-files' tells not to run job on any file (even .py) under neutron/tests/unit/. Also, many members are not aware of these combinations and expected behaviour. It will be helpful if we can have such information with examples in the doc too - https://zuul-ci.org/docs/zuul/latest/config/job.html#attr-job.files
Also, another point is that if Zuul converts these warnings to errors then this will have another big impact on OpenStack where things were working in the past and ended up in error. Can we keep Zuul more backward compatible especially for the things already being in used so many places?
Zuul is intentionally taking this slow as we knew this would be impactful to openstack and others. The backward compatibility path is warning projects that this will become an error giving people plenty of time to convert their rules. That said eventually we do need to stop using the builtin regex library.
This is a deprecation phase path for incompatible changes and not 'avoid incompatible changes/support backward compatibility'. I do not know why Zuul has to stop supporting built-in regex but if there is no strong/broken reason then I will not change the support because it breaks many (or most of them ) repos of OpenStack. Especially when we are struggling with the contributor's bandwidth. -gmann
-gmann
We are landing definitions that will list all files to ignore
explicitly:
https://review.opendev.org/c/openstack/neutron/+/921309
I suspect this could be cleaned up to reuse a single variable
https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/921330 listing files to ignore through */& syntax. >
Thanks,
-Brian
[0] https://review.opendev.org/q/topic:"files-negate" [1] https://github.com/google/re2/issues/156
[2] https://review.opendev.org/c/openstack/ironic/+/921319/1/zuul.d/ironic-jobs....
On Fri, Jun 7, 2024, at 10:13 AM, Ghanshyam Mann wrote:
---- On Wed, 05 Jun 2024 12:05:45 -0700 Clark Boylan wrote ---
On Wed, Jun 5, 2024, at 10:38 AM, Ghanshyam Mann wrote:
---- On Tue, 04 Jun 2024 18:15:49 -0700 Ihar Hrachyshka wrote
Hi Neutrinos,
It was noticed today by Mohammed Naser that the neutron gate was only running the docs and pep8 jobs. We were able to track it down to
patches that were recently merged [0]. Until the reverts land
On Tue, Jun 4, 2024 at 4:52 PM Clark Boylan cboylan@sapwetik.org> wrote: On Tue, Jun 4, 2024, at 1:06 PM, Brian Haley wrote: the RE2 please
don't +W anything since it could merge untested and would just need to be reverted.
Other projects might have similar issues if they updated code using the 'negate' directive for regex recently as we found it doesn't actually work with RE2 when using negative lookahead [1].
Note this is very misleading. It is true that re2 does not support negative lookahead. Zuul's implementation does a positive match then negates the result. This is why you have to say things like regex: foo negate: true. This works outside of typical regex syntax to negate the result.
I think neutron's problem was assuming that negate will be in-place replacement for the use case. Now I don't see how it could in this specific scenario (where irrelevant-files filters are ORed.) The problem here is not re2 or negative lookahead regexes themselves but instead a conversion to non equivalent regular expressions. If we look at an ironic example [2] the old working regex with native negative lookahead support said "all files in tools/ except for bandit.yml are an irrelevant file". The new converted regex said "all files but tools/bandit.yml are an irrelevant file". This meant only changes including updates to tools/bandit.yml would trigger the job.
Yeah, and that is why "negate=true" was considered as direct replacement of "?^" negative regex for zuul config which make it wrong when used in 'irrelevant-files' or 'files'
No, this isn't necessarily wrong for irrelevant-files or files. It depends on what you are trying to achieve. You need to write regex rules that match the files appropriately for your goals. That may end up using negate: true. This is why I said the previous statement is misleading. It made it sound like negate: true is fundamentally broken. It isn't; it does what it says on the tin. Unfortunately, that wasn't what you intended it to do.
We do still need to drop the use of native negative lookaheads
in
zuul configs as re2 regexes will be the only valid regexes in future zuul releases. Quick reverts are a good initial step, but I don't want to have an impression this is something we can revert then leave as is.
So what is the solution here? adding all files explicitly in either 'irrelevant-files' or 'files' does not look good as it can end up having a very lengthy file or need updates every new file is added in that directory. And we cannot add both 'irrelevant-files' or 'files' together.
For Neutron it appears that the intention of those irrelevant files rules is to keep jobs from running when source code is not modified. I made a suggestion in IRC today that Neutron update their jobs to do something like:
files: - ^.*\.py - zuul.d/project.yaml irrelevant-files: - ^neutron/tests/unit/.*$ - ^neutron/tests/fullstack/.*$ - other overmatch exclusion rules here
This matches the intended behavior of "only run this job when source code is changed" much better than managing a massive amount of irrelevant files rules. In the ironic case they decided they could just run bandit jobs more often. I think this is also a valid approach as we often try to over optimize these rules and confuse ourselves (the current situation is an excellent example of this).
Oh, I think I was told/or read somewhere that using both together 'files' and 'irrelevant-files' is unknown behaviour. It may or may not work as expected. If it is confirmed/tested behaviours from Zuul that we can mention both then this will help.
I'm not aware of statements to that effect, but they may have been made. If you look at the code: https://opendev.org/zuul/zuul/src/branch/master/zuul/model.py#L3439-L3448 relevant files are checked first and if they don't match things short circuit. If they do match then irrelevant files is checked next. I think this means you can use irrelevant files to filter over matching done by relevant files.
In the above example, which one takes precedence ('files' or 'irrelevant-files') where there is overlap? What happens if any change in .py file under neutron/tests/unit/ ? 'files' tells run the job on any .py file and 'irrelevant-files' tells not to run job on any file (even .py) under neutron/tests/unit/.
Also, many members are not aware of these combinations and expected behaviour. It will be helpful if we can have such information with examples in the doc too - https://zuul-ci.org/docs/zuul/latest/config/job.html#attr-job.files
Also, another point is that if Zuul converts these warnings to
then this will have another big impact on OpenStack where things were working in the
errors past
and ended up in error. Can we keep Zuul more backward compatible especially for the things already being in used so many places?
Zuul is intentionally taking this slow as we knew this would be impactful to openstack and others. The backward compatibility path is warning projects that this will become an error giving people plenty of time to convert their rules. That said eventually we do need to stop using the builtin regex library.
This is a deprecation phase path for incompatible changes and not 'avoid incompatible changes/support backward compatibility'. I do not know why Zuul has to stop supporting built-in regex but if there is no strong/broken reason then I will not change the support because it breaks many (or most of them ) repos of OpenStack. Especially when we are struggling with the contributor's bandwidth.
Because the normal regex library is inefficient and a potential source of denial of service attacks. Just like OpenStack makes changes for security reasons that are impactful to deployers or end users Zuul has to make similar hard decisions at times too: https://lists.openstack.org/archives/list/openstack-discuss@lists.openstack.... https://lists.openstack.org/archives/list/openstack-discuss@lists.openstack.... I don't think it is fair to hold Zuul to a different standard than the one you hold OpenStack to.
-gmann
-gmann
We are landing definitions that will list all files to ignore
explicitly:
https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/921330
https://review.opendev.org/c/openstack/neutron/+/921309
I suspect this could be cleaned up to reuse a single variable listing files to ignore through */& syntax. >
Thanks,
-Brian
[0] https://review.opendev.org/q/topic:"files-negate" [1] https://github.com/google/re2/issues/156
[2]
https://review.opendev.org/c/openstack/ironic/+/921319/1/zuul.d/ironic-jobs....
UPD: I believe we can merge neutron repo patches again now since https://review.opendev.org/c/openstack/neutron/+/921309 merged. On Tue, Jun 4, 2024 at 4:14 PM Brian Haley <haleyb.dev@gmail.com> wrote:
Hi Neutrinos,
It was noticed today by Mohammed Naser that the neutron gate was only running the docs and pep8 jobs. We were able to track it down to the RE2 patches that were recently merged [0]. Until the reverts land please don't +W anything since it could merge untested and would just need to be reverted.
Other projects might have similar issues if they updated code using the 'negate' directive for regex recently as we found it doesn't actually work with RE2 when using negative lookahead [1].
Thanks,
-Brian
[0] https://review.opendev.org/q/topic:"files-negate" [1] https://github.com/google/re2/issues/156
participants (4)
-
Brian Haley
-
Clark Boylan
-
Ghanshyam Mann
-
Ihar Hrachyshka