Comments (16)
I think this is a good idea. We have to be able to reason in isolation, and shouldn't be relying on remembering what got deployed to what server in what order etc. The fact is that if the MP PR under discussion got merged and deployed then there would be a crash, right? Seems like a simple win/win to me - it doesn't add complexity, that complexity is already there, it just provides a formal mechanism for dealing with it safely.
from readme.
If possible, I'd like to expand this RFC to generalize it from schema changes to all changes. (Otherwise I'd have to make a separate RFC π.)
The GraphQL validation script enforces this for schema changes. A human process should enforce the same expectation when it comes to other dependencies. I.e., we should model and encourage pull requests to only be opened when their prerequisites are already in production. (This brings the same benefits described above of deploys being frequent, low-risk, and fast.)
Of course, [WIP]
PRs are still acceptable for early feedback or whatever, but they should be clearly labeled as such and/or include blocking failures.
from readme.
I.e., we should model and encourage pull requests to only be opened when their prerequisites are already in production.
This (anecdotally in my own experience) differs from the 'ideal' developer workflow. Which is, to make some backend/MP changes, in concert with a front-end update. Sometimes you don't exactly know if everything is truly ready to go on the backend/MP as you're working on the front-end (Reaction component). Then, when you have something working on the front-end, and you're more confident in your backend/MP changes, they're then generally ready to PR at the same time.
So requiring the backend/MP (and within that group, backend and then MP) to be deployed to production before any front-end PR that consumes it feels a bit less than ideal, and adds some friction.
If people do want that, then π all the better, I just wanted to point out that the ideal workflow (IMO) conflicts with this slightly, so we should figure out a way to enforce/encourage this. Otherwise it might be hard to keep that discipline up.
from readme.
@mzikherman would designating those downstream PRs as works-in-progress (i.e., unmergeable) work? Or does the ideal workflow require that those PRs be merged?
from readme.
+1 to this!
I think solid next steps would be to do:
- Add comparable checks in Reaction and Emission (to protect end-clients)
- Add a check to MP
I think there's slightly less risk to the MP side ATM as at least Exchange has some tooling to enforce _schema.yml
updates, so I would prioritize Reaction and Emission first.
from readme.
I'm all for ratifying this deployment philosophy, but I think there would need to be some tooling / workflow to support this. Otherwise, I think it would incredibly hard to operationalize behavior change. The fact that the graphql ecosystem has built tooling like findBreakingChanges
makes me feel more confident that we can actually operationalize this philosophy for graphql-based dependencies.
@joeyAghion Do you have any thoughts on how this philosophy could be operationalized? I think planning rigor in JIRA could be an aid here and services like Horizon help a ton, but nothing comes to mind as some sort of definitive assurance check.
from readme.
Do you have any thoughts on how this philosophy could be operationalized?
We should have all the tools necessary to do it on CI (local schemas, tagged releases) that can let Peril or Danger figure it out at PR time.
from readme.
We should definitely enforce what can be done via tools. As far as non-schema dependencies, I first wanted to see if there was agreement about the goal. It sounds like there is, except for @mzikherman's observation about how it hampers the ideal dev workflow. Matt, see my question above about that.
If there's consensus, we should at least update the playbook to explicitly state the expectation that prerequisite releases are complete before PRs can be merged.
I think it is possible to set an example about this and casually enforce it via PR comments like:
This should be
[WIP]
until ... is released.
Or, harsher:
Closed pending ... release.
Or, punnier:
Don't forget PreRequisites when making a PR
from readme.
I'm π on the schema change requirements; I'm also π on the changes Joey mentioned as long as they're automated by tools. I think we should prioritize getting the schema requirements set up, though.
from readme.
@joeyAghion designating as WIP is great! (My #minor point was about wanting to open PRs at the same time, designating some as blocked/WIP is perfect for that. If that could somehow be automated, ie- if you write 'depends on ...' in the body maybe a WIP label applied cough danger, etc.).
π on all this.
from readme.
Sorry to be late in this, but we've been practicing this for Exchange -> MP -> Reaction -> Force π and has been really useful so all πfor this.
One interesting side-effect of this is also we end up deploying downstream services more often.
from readme.
I feel like I have half the picture and I want to see a full dependency graph.
Can we spec implementation more clearly in terms of the operations we want to perform for a given repo?
I.e. in terms of Metaphysics, on PRs to master we should:
- Check the proposed schema changes against Exchange production introspection API
- Check the proposed schema changes against Gravity production introspection API
... and so on, for all dependent services.
@ashkan18 in this sense I am not following you when you say you are practicing this for "Exchange -> MP -> Reaction -> Force" - can you point to me to this implementation?
In any case let's try and get artsy/force#3061 working first, then come back to a Metaphysics implementation once well defined
from readme.
We're doing this on local discovery now, too - it's not really a formal system exactly it's just that if a feature we need says "Add field X to model Y" then we create tickets to make the change in gravity, then make the change in MP, then downstream systems - and we set up the is blocking
/is blocked by
relations in JIRA to capture this.
The real question is, to what extent can this be automated? Can we catch errors in e.g. whether arguments are expressed in camelCase or snake_case? How much can we enforce via linting? Etc.
Discussing in the platform practice today the notion of grabbing the schema associated with a given commit hash makes some sense, but I'm not sure it captures all cases. As I understand it, we essentially have a KV map where the K is a commit hash and the V is the string schema as of that commit. But given that we're deploying these systems independently, how do we say that a given MP PR requires Gravity to be on commit X and Exchange to be on commit Y? Presumably both gravity and exchange could be on future commits, or could have been rolled back, right? So it feels like we still lack the ability to describe the system at large, or to enforce an RFC like this fully programmatically?
from readme.
Now that Force has implemented validating its schema against Metaphysics staging on merges to master, and against production on merges to release, I would suggest that we roll out this pattern to other applications downstream from Metaphysics and validate upstream... i.e. downstream applications like Force, Exchange, Kaws, Gravity adopt the same pattern.
Validating downstream i.e. Metaphysics validating itself against all its downstream services would IMHO put us closer to a dependency-hell like @mbilokonsky describes - a k/v map of commit hash to the schema at that commit.
from readme.
@izakp this has mostly been through PR process and pretty manual. Meaning when I make a MP change that depends on the Exchange change, we make sure the MP PR is WIP till Exchange has been deployed to production, once that gets deployed, we remove WIP from that PR.
from readme.
I think discussion this RFC is pretty much at a stalled point, some work got put on the platform roadmap. So, I'm going to close this up.
Resolution
We did some of this notably the most critical part of force/metaphysics.
Level of Support
2: Positive feedback.
Additional Context:
Interesting questions about the scope of this changes, and overall feasibility.
Next Steps
N/A
Exceptions
N/A
from readme.
Related Issues (20)
- RFC: Implement Dependency Rotation HOT 8
- [RFC] Feedback Friday time reschedule HOT 2
- RFC: Catch more WTFs during onboarding HOT 2
- RFC: Protect main/master branches HOT 5
- RFC: We are all solely responsible for ensuring that we are not disturbed outside of working hours HOT 16
- RFC: Incrementally adopt I18n library in Rails projects HOT 11
- RFC: Adopt Codecov at Artsy, starting with Gravity HOT 8
- RFC: Adopt inclusive language for repository naming as well as allow/deny lists HOT 12
- RFC: Rename product slack channels to `prd-*` HOT 17
- RFC: Host one Hackathon per quarter in 2022 HOT 8
- RFC: Host one Codebase Refinement per quarter in 2022 HOT 11
- RFC: Officially recommend against using GraphQL Stitching in Gravity HOT 19
- RFC: Reusable components HOT 21
- RFC: Updating Best Practices Documentation HOT 10
- RFC: Retiring Torque HOT 1
- RFC: Feature Flags Naming Conventions / Maintenance HOT 14
- RFC: disallow squashing and rebasing on PRs HOT 17
- Want access of Web & Mobile best practices documentation
- RFC: More Relaxed CodePush Usage for Folio HOT 4
- RFC: Consolidate Eigen feature flags HOT 22
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
π Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. πππ
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google β€οΈ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from readme.