search

Petersoj / alpaca-java

A Java API for Alpaca, the commission free, algo friendly, stock trading broker. https://alpaca.markets
https://petersoj.github.io/alpaca-java/
MIT License
198 stars 84 forks source link

Update Alpaca and Polygon API for new v2 endpoints #21

Closed Petersoj closed 4 years ago

Petersoj commented 4 years ago

This PR updates all the Alpaca and Polygon endpoints along with adding optimizations and organizations to alpaca-java.

~@mainstringargs There seems to be a lot of dynamically generated JSON objects that would come from various responses in the /v2/account/activities endpoint because there is a lot of various activity updates that can happen to an account as shown here. Any ideas on how we should implement such a dynamic endpoint with static JSON schema sources? I'm thinking that we just pass on the JsonObject and the user of this library can get the data they need themselves. That would be the easiest, but not so elegant, approach. Let me know what you think.~

mainstringargs commented 4 years ago

Interesting @Petersoj -- I hadn't looked at this yet. I may be incorrect, but isn't there really just 2 types of json responses -- TradeActivity and NonTradeActivity? The TradeActivity will always have an activity_type of FILL and the NonTradeActivity can have any of these as the activity_type: https://docs.alpaca.markets/api-documentation/api-v2/account-activities/#activity-types? What I'm saying is that each activity type doesn't really have its own information, it will just be populating a NonTradeActivity object (with some fields unset for some activity types -- like the last 2 are only relevant for dividends). Is that correct or am I missing the boat?

Petersoj commented 4 years ago

@mainstringargs Yes you are correct. I was testing this endpoint yesterday and thought I saw objects with different values, but testing it again shows that the JSON returned does indeed correspond with the JSON in the docs. Depending on the activity type, the API will simply return JSON for a Non-Trade Account Activity Entity and omit the key value pairs that aren't used by that particular activity type. Thanks for taking a look at this!

mainstringargs commented 4 years ago

Hey -- is this one still in progress? Thinking I'll push a new 5.0.0 release once this is ready to go

Petersoj commented 4 years ago

@mainstringargs Yes both my open PRs are in progress right now. They should be complete by the end of this weekend hopefully. I think we should update all endpoints for Polygon and Alpaca and do some testing and then release 5.0.0.

mainstringargs commented 4 years ago

Sounds good!

On Thu, Oct 31, 2019 at 8:42 AM Jacob Peterson notifications@github.com wrote:

@mainstringargs https://github.com/mainstringargs Yes both my open PRs are in progress right now. They should be complete by the end of this weekend hopefully. I think we should update all endpoints for Polygon and Alpaca and do some testing and then release 5.0.0.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mainstringargs/alpaca-java/pull/21?email_source=notifications&email_token=ABVXM4XD2AH6L7TJ35QMWBLQRLOD7A5CNFSM4JGS3MHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECX2BWI#issuecomment-548380889, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVXM4XQ5O7LA2XVNI7CJKLQRLOD7ANCNFSM4JGS3MHA .

Petersoj commented 4 years ago

@mainstringargs Looks like the update to v2 is going to take a little longer than I though as I wasn't able to finish this weekend. I'm implementing many optimizations and organizations in addition to updating everything to v2 so there will be quite a few bugs, but those will get ironed out when extensive testing is done 👍

mainstringargs commented 4 years ago

No problem. Thanks for the update! Take your time.

On Sun, Nov 3, 2019 at 6:24 PM Jacob Peterson notifications@github.com wrote:

@mainstringargs https://github.com/mainstringargs Looks like the update to v2 is going to take a little longer than I though as I wasn't able to finish this weekend. I'm implementing many optimizations and organizations in addition to updating everything to v2 so there will be quite a few bugs, but those will get ironed out when extensive testing is done 👍

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mainstringargs/alpaca-java/pull/21?email_source=notifications&email_token=ABVXM4U3VSEU6OIPSTQT25DQR5MRFA5CNFSM4JGS3MHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC6A4ZA#issuecomment-549195364, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVXM4XXO3HEF4MEIB25QP3QR5MRFANCNFSM4JGS3MHA .

Petersoj commented 4 years ago

@mainstringargs This is ready for review finally! I've made a number of changes including optimizations, organizations, and updates:

When the market opens tomorrow, I'll do extensive testing and make sure there are no issues, then this should be good to merge. It would be great if you could review/test all this too. Thanks!

mainstringargs commented 4 years ago

Great work! I’ll review it, but won’t be for a couple days

On Thu, Nov 28, 2019 at 11:58 AM Jacob Peterson notifications@github.com wrote:

@mainstringargs https://github.com/mainstringargs This is ready for review finally! I've made a number of changes including optimizations, organizations, and updates:

  • Make request, exception, and websocket classes abstract.
  • Update all Alpaca and Polygon endpoints
  • Make all JSON sources schemas for jsonschema2pojo (to allow for more flexibility in structure, naming, and documentation of JSON sources)
  • Other refactoring and optimizations

When the market opens tomorrow, I'll do extensive testing and make sure there are no issues, then this should be good to merge. It would be great if you could review/test all this too. Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mainstringargs/alpaca-java/pull/21?email_source=notifications&email_token=ABVXM4SLRQL7WHVAHTZQLDTQWABEFA5CNFSM4JGS3MHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFNGZRA#issuecomment-559574212, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVXM4SFOARDOXN66D6OGFTQWABEFANCNFSM4JGS3MHA .

Petersoj commented 4 years ago

I've tested many things on my end and it all is working great! This should be good to merge and integrated into the new version release. With schemas and the abstraction implemented, it'll be easier to implement new API changes with Alpaca and Polygon so we can keep this library up-to-date and consistent.

mainstringargs commented 4 years ago

I pulled in the latest, and I'm seeing this: image

Petersoj commented 4 years ago

@mainstringargs Try my new push. Sorry for all the debugging. I don't have access to a Windows machine...

mainstringargs commented 4 years ago

Getting warmer -- now I'm getting another error:

image

Thinking its a cloning issue? Or are you seeing this too?

Petersoj commented 4 years ago

My build has been building successfully this entire time with no warnings haha. This is a file encoding issue. All the file encodings are in UTF-8, but it looks like your computer's default encoding for the gradle javadoc plugin is something else (looks like Cp1252 which I've never heard of). I've explicitly set the encoding for the javadoc plugin in my latest push. Try it now with gradlew clean build. Again, sorry for the debugging! I appreciate your fast responses!

mainstringargs commented 4 years ago

Ah! There we go... Built this time! I'll take a closer look now. Thanks!!

Petersoj commented 4 years ago

Hey don’t merge this quite yet! I’ve found a bug with order requesting. Will fix soon!

mainstringargs commented 4 years ago

No problem. I’ll review a bit more, then I’ll merge tomorrow unless you need me to hold it for any reason

On Mon, Dec 2, 2019 at 6:38 PM Jacob Peterson notifications@github.com wrote:

Hey don’t merge this quite yet! I’ve found a bug with order requesting. Will fix soon!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mainstringargs/alpaca-java/pull/21?email_source=notifications&email_token=ABVXM4UNFEPQKDM434HQOZDQWWTBBA5CNFSM4JGS3MHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFXVOJI#issuecomment-560944933, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVXM4Q7VMPIJLLODT74PTTQWWTBBANCNFSM4JGS3MHA .

Petersoj commented 4 years ago

@mainstringargs I've fixed the problem with submitting market orders (a precondition check was causing that method to fail on Market orders) and I've added overload methods for the various types of order requesting for parameter simplicity (e.g. might be confusing for users who have an OrderType.MARKET to have limitPrice and stopPrice in the parameter list). Everything looks good to be merged (hopefully)! Remember to update the javadoc link and the version in the usage section of the README. Thanks! After this big merge, I think bug fixes should be rolled out more regularly (as in uploaded to maven central ASAP) as there is a surprising number of individuals who use this library :)

mainstringargs commented 4 years ago

Cool. Great job! I will do all of that and push up a 5.0.0 release tomorrow after I test it out once the market opens up.

On Mon, Dec 2, 2019 at 9:47 PM Jacob Peterson notifications@github.com wrote:

@mainstringargs https://github.com/mainstringargs I've fixed the problem with submitting market orders (a precondition check was causing that method to fail on Market orders) and I've added overload methods for the various types of order requesting for parameter simplicity (e.g. might be confusing for users who have an OrderType.MARKET to have limitPrice and stopPrice in the parameter list). Everything looks good to be merged (hopefully)! Remember to update the javadoc link and the version in the usage section of the README. Thanks! After this big merge, I think bug fixes should be rolled out more regularly (as in uploaded to maven central ASAP) as there is a surprising number of individuals who use this library :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mainstringargs/alpaca-java/pull/21?email_source=notifications&email_token=ABVXM4QABTV2UMHMLHI5HP3QWXJDJA5CNFSM4JGS3MHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFX7NYA#issuecomment-560985824, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVXM4URF4YL3JMM6IDD7XTQWXJDJANCNFSM4JGS3MHA .