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.

Sign up for GitHub

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

Jump to bottom

Add news api #555

Merged
merged 7 commits into from
Jan 26, 2022
Merged

Add news api #555

merged 7 commits into from
Jan 26, 2022

Conversation

ccnlui
Copy link
Contributor

@ccnlui ccnlui commented Jan 10, 2022 •
edited

Context

Added support for news api, both stream and historical endpoint

TODO

  • Add tests (done)

@OlegRa OlegRa mentioned this pull request Jan 11, 2022
Closed
1 task
@ccnlui ccnlui requested a review from a team January 11, 2022 16:16
gnvk
gnvk reviewed Jan 12, 2022
View reviewed changes
alpaca_trade_api/entity_v2.py Outdated Show resolved Hide resolved
gnvk
gnvk reviewed Jan 12, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
@gnvk gnvk changed the title ENG-8782 add news api Add news api Jan 12, 2022
gnvk
gnvk reviewed Jan 12, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
gnvk
gnvk reviewed Jan 12, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
gnvk
gnvk reviewed Jan 12, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
gnvk
gnvk reviewed Jan 12, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
gnvk
gnvk reviewed Jan 12, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
gnvk
gnvk reviewed Jan 12, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
gnvk
gnvk reviewed Jan 12, 2022
View reviewed changes
alpaca_trade_api/stream.py Outdated Show resolved Hide resolved
ccnlui
ccnlui commented Jan 12, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
@ccnlui
Copy link
Contributor Author

ccnlui commented Jan 12, 2022

For get_news(), I'm using NEWS_MAX_LIMIT = 50 as a default query limit to prevent fetching too many news articles. no_total_limit can be used to suppress this.

It also acts as a page_limit to minimize api calls for the client if total_limit is more than 50. This mimics behaviour of the other endpoints like get_trades()

The difference is that the other endpoints don't have a default query limit... So get_trades('AAPL') would actually just fetch everything, 10000 data points at a time...

I wonder if I should add total_limit and no_total_limit to the other methods too, but it's a breaking change 🤔

@ccnlui ccnlui requested a review from gnvk January 12, 2022 22:19
gnvk
gnvk reviewed Jan 13, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
gnvk
gnvk reviewed Jan 13, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
gnvk
gnvk reviewed Jan 13, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
gnvk
gnvk reviewed Jan 13, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
gnvk
gnvk reviewed Jan 13, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
gnvk
gnvk reviewed Jan 13, 2022
View reviewed changes
alpaca_trade_api/rest.py Outdated Show resolved Hide resolved
gnvk
gnvk approved these changes Jan 13, 2022
View reviewed changes
Copy link
Contributor

@gnvk gnvk left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @ccnlui!

@gnvk gnvk merged commit 6132575 into master Jan 26, 2022
@gnvk gnvk deleted the ENG-8782-add-news branch January 26, 2022 10:45
fumoboy007 added a commit to fumoboy007/alpaca-trade-api-python that referenced this pull request Mar 27, 2023
@fumoboy007
Fix streaming news article event dispatching.
847829c 
News article events are different from other types of events (e.g. stock trades) because a single news article could be applicable to multiple symbols. For that reason, the streamed event does not have the usual `S` field that contains a single symbol; instead, the event has a `symbols` field that contains a list of symbols.

alpacahq#555 mistakenly assumes that the event has an `S` field. This commit changes the dispatching logic to instead use the `symbols` field.

(There is a similar [commit](alpacahq/alpaca-trade-api-csharp@be67b62) for the C# library.)
@fumoboy007 fumoboy007 mentioned this pull request Mar 27, 2023
Merged
fumoboy007 added a commit to fumoboy007/alpaca-trade-api-python that referenced this pull request Mar 27, 2023
@fumoboy007
Fix streaming news article event dispatching.
0fc8c39 
News article events are different from other types of events (e.g. stock trades) because a single news article could be applicable to multiple symbols. For that reason, the streamed event does not have the usual `S` field that contains a single symbol; instead, the event has a `symbols` field that contains a list of symbols.

alpacahq#555 mistakenly assumes that the event has an `S` field. This commit changes the dispatching logic to instead use the `symbols` field.

(There is a similar [commit](alpacahq/alpaca-trade-api-csharp@be67b62) for the C# library.)
fumoboy007 added a commit to fumoboy007/alpaca-trade-api-python that referenced this pull request Mar 27, 2023
@fumoboy007
Fix streaming news article event dispatching.
9ef0513 
News article events are different from other types of events (e.g. stock trades) because a single news article could be applicable to multiple symbols. For that reason, the streamed event does not have the usual `S` field that contains a single symbol; instead, the event has a `symbols` field that contains a list of symbols.

alpacahq#555 mistakenly assumes that the event has an `S` field. This commit changes the dispatching logic to instead use the `symbols` field.

(There is a similar [commit](alpacahq/alpaca-trade-api-csharp@be67b62) for the C# library.)
fumoboy007 added a commit to fumoboy007/alpaca-trade-api-python that referenced this pull request Mar 27, 2023
@fumoboy007
Fix streaming news article event dispatching.
e5046b8 
News article events are different from other types of events (e.g. stock trades) because a single news article could be applicable to multiple symbols. For that reason, the streamed event does not have the usual `S` field that contains a single symbol; instead, the event has a `symbols` field that contains a list of symbols.

alpacahq#555 mistakenly assumes that the event has an `S` field. This commit changes the dispatching logic to instead use the `symbols` field.

(There is a similar [commit](alpacahq/alpaca-trade-api-csharp@be67b62) for the C# library.)
fumoboy007 added a commit to fumoboy007/alpaca-trade-api-python that referenced this pull request Mar 27, 2023
@fumoboy007
Fix streaming news article event dispatching.
19198b5 
News article events are different from other types of events (e.g. stock trades) because a single news article could be applicable to multiple symbols. For that reason, the streamed event does not have the usual `S` field that contains a single symbol; instead, the event has a `symbols` field that contains a list of symbols.

alpacahq#555 mistakenly assumes that the event has an `S` field. This commit changes the dispatching logic to instead use the `symbols` field.

(There is a similar [commit](alpacahq/alpaca-trade-api-csharp@be67b62) for the C# library.)
gnvk pushed a commit that referenced this pull request Mar 27, 2023
@fumoboy007
Fix streaming news article event dispatching. (#691)
5c2d74a 
News article events are different from other types of events (e.g. stock trades) because a single news article could be applicable to multiple symbols. For that reason, the streamed event does not have the usual `S` field that contains a single symbol; instead, the event has a `symbols` field that contains a list of symbols.

#555 mistakenly assumes that the event has an `S` field. This commit changes the dispatching logic to instead use the `symbols` field.

(There is a similar [commit](alpacahq/alpaca-trade-api-csharp@be67b62) for the C# library.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gnvk gnvk gnvk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ccnlui @gnvk