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

Watch: Refactor GraphQL subscription handling #868

Closed
ansgarm opened this issue Jul 30, 2021 · 2 comments
Closed

Watch: Refactor GraphQL subscription handling #868

ansgarm opened this issue Jul 30, 2021 · 2 comments
Labels
feature/watch needs-research priority/awaiting-more-evidence Lowest priority. Unlikely to be worked on unless/until it gets a lot more upvotes. tech-debt

Comments

@ansgarm
Copy link
Member

ansgarm commented Jul 30, 2021

Currently the subscription handling comes with a few drawbacks which we should address

// subscribe returns an unsubscribe function but that connect reliably
// be used with PubSub – that lib is recommended by Apollo docs but seems to
// be unmaintained, maybe ditch it? see https://github.com/apollographql/graphql-subscriptions/issues/240#issuecomment-767568596

// we should get rid if this hack somehow.
// this has to be done because the client won't receive this
// error if it is published before the iterator has been returned

Background

The initial implementation of the new cdktf watch command also contains an
experiment in regards to separating the underlying invocation of a synth and of
Terraform for init, plan and apply from the presentation layer (which is in our
case a CLI build with React components using ink). The other commands (e.g.
cdktf deploy) which we currently have, make use of React Hooks for
orchestrating the underlying steps that are required. Quite early it turned out
to be unreasonable to implement the dynamic nature of the watch command using
those same tools. Instead we opted for trying out a separate process and using a
GraphQL api to connect the two. The main reason for this (as opposed to e.g.
ProtoBuf or REST) was the superior tooling support that exists for this in
TypeScript and React. However, with this prototype now in the open, we will
carefully examine it and assess the pros and cons before jumping ship.

(from #817)

@ansgarm ansgarm added needs-research needs-priority Issue has not yet been prioritized; this will prompt team review feature/watch labels Jul 30, 2021
@danieldreier danieldreier added priority/awaiting-more-evidence Lowest priority. Unlikely to be worked on unless/until it gets a lot more upvotes. tech-debt and removed needs-priority Issue has not yet been prioritized; this will prompt team review labels Sep 2, 2021
@ansgarm
Copy link
Member Author

ansgarm commented Apr 4, 2022

#1658 removed GraphQL, so this is obsolete 🎉

@ansgarm ansgarm closed this as completed Apr 4, 2022
@github-actions
Copy link
Contributor

I'm going to lock this issue because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature/watch needs-research priority/awaiting-more-evidence Lowest priority. Unlikely to be worked on unless/until it gets a lot more upvotes. tech-debt
Projects
None yet
Development

No branches or pull requests

2 participants