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

update rest snapshot models, add FMV data types and new feed urls for websockets #356

Merged
merged 14 commits into from
Oct 25, 2023

Conversation

aitzkovitz
Copy link
Contributor

No description provided.

Copy link
Contributor

@morningvera morningvera left a comment

Choose a reason for hiding this comment

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

do we need to update the websocket spec too?

rest/models/snapshot.go Show resolved Hide resolved
@aitzkovitz
Copy link
Contributor Author

do we need to update the websocket spec too?

yeah i believe @aliche-a has a PR for the ws specs update somewhere? I think we're waiting to release that

@morningvera
Copy link
Contributor

@aitzkovitz hmm i don't a PR or commit with the updated ws spec

@aitzkovitz
Copy link
Contributor Author

@aitzkovitz hmm i don't a PR or commit with the updated ws spec

@aitzkovitz hmm i don't a PR or commit with the updated ws spec

Hmm i was looking at this issue and saw that it was reference by another issue titled [WIP] Update Launchpad Websocket specs #332. But that link is broken, so maybe that work hasn't ben done yet. Do you think this PR depends on that work?

@morningvera
Copy link
Contributor

@aitzkovitz yeah if you're updating the websocket client, you should update the spec too, it's just a simple command in the makefile

@aitzkovitz
Copy link
Contributor Author

@aitzkovitz yeah if you're updating the websocket client, you should update the spec too, it's just a simple command in the makefile

Oh i see what you're saying. I ran both the

@aitzkovitz yeah if you're updating the websocket client, you should update the spec too, it's just a simple command in the makefile

Oh i see what you're saying. I ran both the websocket and rest spec update commands originally. But we haven't released any spec changes (we're waiting until next week) so those should look the same aside from any other spec changes which have gone out since the last release.

Copy link
Contributor

@morningvera morningvera left a comment

Choose a reason for hiding this comment

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

@aitzkovitz got it, lgtm!

@aliche-a
Copy link

aliche-a commented Oct 18, 2023

This PR is the one I had to update ws specs, but I made it before we decided not to modify the existing Launchpad value field. I haven't had the chance to go back and update it since, but I should be able to later today.

@aitzkovitz I think we also need to update this handleData method so FMV data can be unmarshalled into the FMV model.

websocket/config.go Outdated Show resolved Hide resolved
@aitzkovitz aitzkovitz force-pushed the ai.support-business-products branch from cd80a6b to af8b909 Compare October 25, 2023 15:52
@aitzkovitz aitzkovitz merged commit 9588ed0 into master Oct 25, 2023
16 checks passed
@aitzkovitz aitzkovitz deleted the ai.support-business-products branch October 25, 2023 20:52
Adam-Mustafa pushed a commit to FinTronners/polygon-client-go that referenced this pull request Mar 16, 2024
… websockets (polygon-io#356)

Co-authored-by: GitHub Action <action@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants