-
-
Notifications
You must be signed in to change notification settings - Fork 225
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_382 #383
fix_382 #383
Conversation
I thought I'd be a nice neighbor and let you know that certain regions (probably EU) seem to get redirected to a consent page during the initial call, thus failing to obtain a session. We're currently investigating possible solutions: |
thx @pstadler. it looks like the cookies and crumbs are no longer needed again for v7 (at least for now). r u seeing the redirects on calls to the v7 api w/out simulating a session? |
it‘s happening over at my project using something very similar to your fix. Going to remove it again if it‘s working again without crumb. Wondering for how long though, it‘s been flaky for a while. |
I gladly reverted the change on my end. Fingers crossed that it won't happen again. |
im currently testing an update that caches the yahoo handle. i dont know what the error will ben when the handle becomes invalid, as this is probably driven by a timeout on the yahoo server side, so i cant force the error condition. will push once i have that part worked out |
my assumption is that you‘ll get a 401 Unauthorized if the session expires (if this is what you’re referring to). In this case I‘d simply try to obtain a new session and fetch the quotes again. See /~https://github.com/pstadler/ticker.sh/blob/54b73a3f57432b5efd102754c48f20ed246f8ed2/ticker.sh#L53 |
added handle caching across calls
This comment was marked as outdated.
This comment was marked as outdated.
if ((r$status_code == 200) && (length(r$content) > 0)) { | ||
ses$crumb = rawToChar(r$content) | ||
} else { | ||
if (!force.new) ses <- .yahooSession(TRUE) else stop("Unable to get yahoo crumb") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems backward. I expect to call .yahooSession(TRUE)
when force.new
is TRUE
, else throw an error because we couldn't get a crumb.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its really a state machine. the force.new
is never intended to be used directly, only in the recursive call. i can rewrite it as a loop to clean up the signature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling the function recursively is fine. The logic on this line (51) confused me, but seems correct once I walk through it.
Assume force.new
is FALSE
. Line 45 checks that we have an active session. Line 51 checks if we're able to get a crumb. If we can't get a crumb, then !force.new
is TRUE
and we call .yahooSession(TRUE)
. So that seems like it does what it should.
I think some comments would help me remember what this is doing.
# we were unable to get a crumb
if (force.new) {
# we couldn't get a crumb with a new session
stop("unable to get yahoo crumb")
} else {
# we tried to re-use a session but couldn't get a crumb
# try to get a crumb using a new session
ses <- .yahooSession(TRUE)
}
### Changes in 0.4.25 (2023-08-21) 1. Fix `getQuote.yahoo()` for API changes. Thanks to Ethan B. Smith for the report and patch! Also add error message for users in GDPR countries, since we cannot automatically consent to GDPR and the request fails without consent. [#392](joshuaulrich/quantmod#392) [#393](joshuaulrich/quantmod#393) [#395](joshuaulrich/quantmod#395) 1. Fix `getQuote.yahoo()` when the user only requested metrics that do not have have a value for 'regularMarketTime'. Set the value to NA in these cases so the output remains the same regardless of whether the endpoint returns a 'regularMarketTime' or not. Thanks to @mehdiMBH for the report! [#255](joshuaulrich/quantmod#255) 1. Add fields to `getQuote.yahoo()` that are returned when no fields are explicitly requested. Thanks to @Courvoisier13 for the report! [#335](joshuaulrich/quantmod#335) 1. Add intraday endpoint to `getSymbols.yahoo()`. Thanks to @kapsner for the report and patch! Also allow suppressing the warning if more than 7 days of data are requested (@eddelbuettel). [#351](joshuaulrich/quantmod#351) [#381](joshuaulrich/quantmod#381) [#399](joshuaulrich/quantmod#399) 1. Add warning if `getSymbols()` is called with tickers that are reserved words because accessing them requires back-quotes (e.g. ``NA``). [#401](joshuaulrich/quantmod#401) 1. Fix `allReturns()` when 'subset' is specified. Thanks to @Panagis1980 for the report! [#402](joshuaulrich/quantmod#402) ### Changes in 0.4.24 (2023-07-17) 1. Fix `getSymbols.oanda()` URL. Thanks to @macray76 for the report. [#387](joshuaulrich/quantmod#387) ### Changes in 0.4.23 (2023-06-14) 1. Fix `getQuote.yahoo()` error. Thanks to Ethan B. Smith for the report and patch! [#382](joshuaulrich/quantmod#382) [#383](joshuaulrich/quantmod#383) 1. Add `name` argument to `add_TA()`. Thanks to @SamoPP for the suggestion! [#377](joshuaulrich/quantmod#377) [#205](joshuaulrich/quantmod#205)
adds a lightweight session for use on yahoo quote requests
session is needed for both the cookies and the "crumb" field
the handle from the session must also be used for subsequent requests
fixes #382