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

Make timeout for Ajax call configurable #166

Merged
merged 7 commits into from
Aug 31, 2023
Merged

Conversation

3link
Copy link
Contributor

@3link 3link commented Aug 30, 2023

Short description if any.

Author Todo List:

  • Add/adjust tests (if applicable)
  • Build in CI passes
  • Latest master revision is merged into the branch
  • Self-Review
  • Set Ready For Review status

Copy link
Contributor

@wi101 wi101 left a comment

Choose a reason for hiding this comment

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

it's draft, but LGTM thanks!

@3link 3link marked this pull request as ready for review August 31, 2023 11:21
@3link 3link requested a review from a team as a code owner August 31, 2023 11:21
Copy link
Contributor

@mschuwalow mschuwalow left a comment

Choose a reason for hiding this comment

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

Let's document this new option

@3link
Copy link
Contributor Author

3link commented Aug 31, 2023

Let's document this new option

Done in 339265e

@3link
Copy link
Contributor Author

3link commented Aug 31, 2023

@3link 3link merged commit 0012579 into master Aug 31, 2023
4 of 5 checks passed
@3link 3link deleted the configurable_ajax_timeout branch August 31, 2023 13:05
@robertrmartinez
Copy link

@3link Awesome!

I see 6.0.1 was released.

If I just build prebid locally and bump the live-connect package to this version I should be good right?

I figure you will make a PR to prebid to do this, but wondering if I can do a patch fix on older versions of prebid in my fork and it should work right?

Thanks for the quick fix!

@robertrmartinez
Copy link

Also just to point out, if one does not use ajaxTimeout setting, then it still defaults to 0

I thought that in this PR you would also have the default updated to something else valid?

Like 3000 or something.

@robertrmartinez
Copy link

Even when using the pbjs liveintent config param ajaxTimeout I get it set to 0

I probably am doing somethign wrong.

image

image

@3link
Copy link
Contributor Author

3link commented Sep 1, 2023

@robertrmartinez For the change to work with Prebid, it requires adjustments in Prebid beyond the LiveConnect version update. This draft in our fork shows how to do it: LiveIntent/Prebid.js#20. A PR in the Prebid repository will follow soon.

@3link
Copy link
Contributor Author

3link commented Sep 1, 2023

@robertrmartinez This is the PR in Prebid: prebid/Prebid.js#10425

@robertrmartinez
Copy link

Sorry! I got too excited.

Thanks for the quick update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants