search

barrywood78 / Coinbase.AdvancedTrade

Coinbase Advanced Trade API Wrapper
8 stars 2 forks source link

Why drop legacy API keys? #15

Closed michielpost closed 5 months ago

michielpost commented 5 months ago

The reason I started using this library was that there is support for both legacy and new API keys. It's great and I can support both keys with one library. Thanks for that.

In the latest release, the legacy API key support is removed. What I understand from the Coinbase communication is that you can no longer create new legacy API keys, but existing legacy API keys will continue to work. So it feels a bit premature to completely remove the legacy API key support. Maybe a better way is to mark it as obsolete and remove it after a while.

barrywood78 commented 5 months ago

@michielpost Hi, I appreciate your comment. I had maybe misunderstood the email I received from Coinbase suggesting that all functionality for legacy keys would no longer work but upon reading it again, you are right and they say they will continue to work.

I will add back the legacy key support today or tomorrow when I have a bit of time.

michielpost commented 5 months ago

Thanks, great to hear!

ravens23og commented 5 months ago

I will say that as a newcomer and having downloaded this repo this weekend, I was happy to see the new dev CBP keys were plug'n'play. Perhaps a fork into a feature branch could also be a path forward. Thanks for the repo btw!

barrywood78 commented 5 months ago

@ravens23og Thanks for the comment :)

Before my latest release last week, the CoinbaseClient had supported both new CDP keys and the Legacy keys by passing in an optional param into the client constructor. I'm reverting back a specific change to provide an optional ApiKeyType (Legacy & CDP(Default)).

I'm currently testing and don't expect a breaking change except possibly for those still using the legacy key as the optional param will no longer be defaulted to Legacy but instead CDP.

barrywood78 commented 5 months ago

@michielpost I have pushed an update just a bit ago to revert the CoinbaseClient back to include an optional parameter called "ApiKeyType". This enum was there previously but defaulted to Legacy. I have now set Legacy as obsolete and have type CoinbaseDeveloperPlatform as the default. Depending on how you had your constructor, this may be a breaking change but you just need to include the parameter.

Thanks!