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

Fix http2 with curl #547

Merged
merged 1 commit into from Oct 19, 2018

Conversation

Projects
None yet
5 participants
@Josue-T
Contributor

Josue-T commented Oct 1, 2018

The problem

  • Curl fail with http2 with wordpress app.

Solution

  • Add a sleep after nginx is reloaded.

By example if we have a code like that it fail :

# Reload Nginx
systemctl reload nginx

# Wordpress installation
ynh_local_curl "/wp-admin/install.php?step=2" "&weblog_title=YunoBlog" "user_name=$admin_wordpress" "admin_password=$db_pwd" "admin_password2=$db_pwd" "admin_email=$admin_wordpress@$domain" "Submit=Install+WordPress"

But this code won't fail :

# Reload Nginx
systemctl reload nginx

# Wait untils nginx has fully reloaded (fix http2 issue)
sleep 2

# Wordpress installation
ynh_local_curl "/wp-admin/install.php?step=2" "&weblog_title=YunoBlog" "user_name=$admin_wordpress" "admin_password=$db_pwd" "admin_password2=$db_pwd" "admin_email=$admin_wordpress@$domain" "Submit=Install+WordPress"

PR Status

Tested with the CI, but need more test.

How to test

  • Checkout the branch
  • Enable http2 in the nginx config
  • Try to install wordpress

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :

@Josue-T Josue-T referenced this pull request Oct 1, 2018

Closed

Fix_http2_support #51

3 of 8 tasks complete
@frju365

This comment has been minimized.

Member

frju365 commented Oct 1, 2018

Well, I 'm not in the list of reviewers... :-P

To be serious, perhaps you can add http2 in nginx config by default if this PR fixed the problem.

@Josue-T

This comment has been minimized.

Contributor

Josue-T commented Oct 1, 2018

Well, I 'm not in the list of reviewers... :-P

Sorry I didn't found you in the list of reviewer (I don't know why). But I thought of you as I know that you do some reseach about that.

To be serious, perhaps you can add http2 in nginx config by default if this PR fixed the problem.

I didn't enabled http2, because I think this could be really easy to merge. Enabling http2 need maybe some more discussion

@frju365

This comment has been minimized.

Member

frju365 commented Oct 1, 2018

mh. Yes, I agree with you. Well, if this PR merged before the next meeting, we will be able to speak of that next one (or another).

@frju365

frju365 approved these changes Oct 1, 2018

@frju365

This comment has been minimized.

Member

frju365 commented Oct 19, 2018

Can we merge this as it was tested already ?

@alexAubin

This comment has been minimized.

Member

alexAubin commented Oct 19, 2018

Yolomergin then

@alexAubin alexAubin merged commit 428a299 into stretch-unstable Oct 19, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@alexAubin alexAubin deleted the Fix_curl_http2 branch Oct 19, 2018

@alexAubin alexAubin modified the milestones: 3.2.x, 3.3.x Oct 19, 2018

@Josue-T Josue-T referenced this pull request Nov 22, 2018

Closed

Update server.tpl.conf #578

0 of 4 tasks complete
@liberodark

This comment has been minimized.

Contributor

liberodark commented Nov 22, 2018

yes 👍 need this for fix that : curl: (16) Error in the HTTP2 framing layer

@frju365 frju365 referenced this pull request Nov 22, 2018

Merged

HTTP2 On #580

0 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment