Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove check of OpenResty in code because APISIX can not start when using an old version of OpenResty #9926

Merged
merged 1 commit into from
Aug 4, 2023

Conversation

ronething
Copy link
Contributor

@ronething ronething commented Jul 29, 2023

Description

Fixes # (issue)

due to this PR #9913 merge, we don't need to check OpenResty support ngx.process in the stream subsystem or not because we use OpenResty version >= 1.21.4.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@moonming
Copy link
Member

@ronething The purpose of this PR is to no longer be compatible with older versions of OpenResty? I think you need a better title of this PR.

@ronething
Copy link
Contributor Author

@moonming Hello,

The purpose of this PR is to no longer be compatible with older versions of OpenResty?

due to this PR #9913 merge, we don't need to check OpenResty support ngx.process in the stream subsystem or not because we use OpenResty version >= 1.21.4.

I think you need a better title of this PR.

maybe change the PR description is enough? if we need to change the PR title, what's better?

Addition, can you approve the workflow?

@moonming
Copy link
Member

maybe change the PR description is enough? if we need to change the PR title, what's better?

Yes. How about chore: Remove check of OpenResty version in code base because APISIX can not start when using an old version of OpenResty

@ronething ronething changed the title chore: remove some useless code about kubernetes and tars stream discovery chore: remove check of OpenResty in code because APISIX can not start when using an old version of OpenResty Jul 31, 2023
@ronething ronething marked this pull request as ready for review July 31, 2023 08:05
@ronething
Copy link
Contributor Author

ronething commented Jul 31, 2023

Yes. How about chore: Remove check of OpenResty version in code base because APISIX can not start when using an old version of OpenResty

title changed, but CI failed. please rerun it when you have time, thanks.

@monkeyDluffy6017
Copy link
Contributor

monkeyDluffy6017 commented Aug 3, 2023

I think you misunderstood this pr: #9913.
Its purpose is to remove the check of the compatibility between APISIX and openresty, because we use apisix-base instead.
And we still have apisix-base that lower than 1.19.9.1, so i think we should keep these code

@ronething
Copy link
Contributor Author

I think you misunderstood this pr: #9913.

Its purpose is to remove the check of the compatibility between APISIX and openresty, because we use apisix-base instead.

And we still have apisix-base that lower than 1.19.9.1, so i think we should keep these code

ok,but i wonder if we use the master branch code with apisix-base version lower than 1.21 after PR #9913,can we start the apisix successfully?

@monkeyDluffy6017
Copy link
Contributor

monkeyDluffy6017 commented Aug 3, 2023

ok,but i wonder if we use the master branch code with apisix-base version lower than 1.21 after PR #9913,can we start the apisix successfully?

I'm not sure.
Maybe we should check the versions of dependencies (apisix-base etc) when installing APISIX.
Each version of APSIX has its corresponding dependencies. And after that we could remove these code.

@monkeyDluffy6017
Copy link
Contributor

@ronething I found that the limitation has been add in #9913.
image
So your pr is acceptable

@soulbird
Copy link
Contributor

soulbird commented Aug 3, 2023

LGTM

@ronething
Copy link
Contributor Author

ronething commented Aug 3, 2023

@monkeyDluffy6017 ok, can you please rerun the cancelled job?

@monkeyDluffy6017 monkeyDluffy6017 merged commit 6278ae0 into apache:master Aug 4, 2023
36 checks passed
@ronething ronething deleted the chore/or_1.19 branch August 4, 2023 14:35
rubikplanet pushed a commit to rubikplanet/apisix that referenced this pull request Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants