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

Fixes #2778 - Implements HTTP/2 Push through Flask #2792

Merged
merged 2 commits into from
Feb 14, 2019

Conversation

marimeireles
Copy link
Member

These are the relevant changes to test the firehose lib reponsible for making push requests on Flask.
Can you please add them to Staging? So we can test if there is something wrong with my local nginx configuration.

Here is some more context to why we're doing this: #2778 (comment)

r? @miketaylr

Thanks!

Copy link
Member

@karlcow karlcow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marimeireles This is good work.

I wonder if there is a conflict in between HTML and HTTP. Maybe as a test we could

  1. remove from the HTML template to see if it's working
  2. Pass the values to the HTML template to see if it's working better.

@miketaylr
Copy link
Member

@karlcow can you (re-)deploy this branch to staging? I tried, but apparently it doesn't like me. The fabfile is asking for a pw, that I guess I don't have (or possibly it changed)?

@karlcow
Copy link
Member

karlcow commented Feb 6, 2019

@karlcow can you (re-)deploy this branch to staging? I tried, but apparently it doesn't like me. The fabfile is asking for a pw, that I guess I don't have (or possibly it changed)?

Probably the issue we talk about last week. I will deploy this branch. It has not been deployed yet.

@karlcow
Copy link
Member

karlcow commented Feb 6, 2019

@miketaylr @marimeireles Firehose lib branch deployed on staging as of now.

@marimeireles
Copy link
Member Author

Thanks for the help Karl and Mike.
I'll finish this PR and ask for your review once it's finished.
I still have to add the other files that have to be pushed and the code that's going to prevent the browser to fetch resources if it already has it stored.

@karlcow
Copy link
Member

karlcow commented Feb 6, 2019

See @marimeireles' comment

@marimeireles marimeireles changed the title Issue #2778 - Tests the firehose lib Issue #2778 - Using flask to push files Feb 7, 2019
@marimeireles marimeireles force-pushed the issues/2778/1 branch 3 times, most recently from 578bd2b to 9f07b7b Compare February 7, 2019 16:01
@marimeireles
Copy link
Member Author

r? @karlcow

@marimeireles marimeireles force-pushed the issues/2778/1 branch 4 times, most recently from e1183f1 to 9b8847a Compare February 7, 2019 23:58
@marimeireles
Copy link
Member Author

Does it look good for you @karlcow ?

@karlcow
Copy link
Member

karlcow commented Feb 12, 2019

The rest looks good to me.

thanks a lot. Once you put back the order of nosetest, I'll approve and merge.

@miketaylr
Copy link
Member

@karlcow are we good to merge here? thanks.

@karlcow
Copy link
Member

karlcow commented Feb 12, 2019

I will double check the config on prod and staging today. And we could probably merge it.

Copy link
Member

@karlcow karlcow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for everything @marimeireles

@karlcow karlcow merged commit 9947b46 into webcompat:master Feb 14, 2019
@karlcow karlcow changed the title Issue #2778 - Using flask to push files Issue #2778 - Implements HTTP/2 Push through Flask Feb 14, 2019
@karlcow karlcow changed the title Issue #2778 - Implements HTTP/2 Push through Flask Fixes #2778 - Implements HTTP/2 Push through Flask Feb 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants