-
Notifications
You must be signed in to change notification settings - Fork 24
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
New Database Trigger Options #611
Conversation
Trigger is associated with. | ||
|
||
- :guilabel:`Database Name`. The MongoDB database that contains the watched | ||
collection. Optional if the :guilabel:`Collection Name` is specified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
collection. Optional if the :guilabel:`Collection Name` is specified. | |
collection. Required if the :guilabel:`Collection Name` is specified. |
Maybe we should add some copy about how a Deployment trigger is when database and collection
are not specified. Database trigger is when collection
is not specified, and Collection Trigger when both are specified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nathantfrank does this mean that even if the user selects the Collection or Database source type, it will actually be a Deployment type if they leave both database
and collection
blank?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh Sorry I misunderstood what this section was doing.
My current thought is we should just remove that sentence.
collection. Optional if the :guilabel:`Collection Name` is specified. | |
collection. |
|
||
.. tip:: Performance Optimization | ||
- :guilabel:`Operation Type`. The :ref:`database operation types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Q] should this be database
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nathantfrank Sorry, not sure what you're asking here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh, I think I'm confusing myself with existing documentation details. I was thinking this should be like collection operation types
and link to something different.
- Drop Database | ||
- Create Collection | ||
- Modify Collection | ||
- Rename Collection | ||
- Shard Collection | ||
- Drop Collection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only Drop Database
is a deployment only operation type. All the rest of these are database operation types.
[Q] Also do we want to add this list to each of them (COLLECTION, DATABASE, DEPLOYMENT)?
- Create Collection | ||
- Modify Collection | ||
- Rename Collection | ||
- Shard Collection | ||
- Drop Collection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in our chat, these are actually Database+ change event types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also just talked to Product about this recent change. Deployment level will not be supported if the cluster being watched is a shared tier. Not exactly sure where we want to add this in the docs.
- Create Document | ||
- Modify Document | ||
- Rename Document | ||
- Shard Document | ||
- Drop Document |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be Insert, Update, Replace, and Delete
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doh!
- Create Collection | ||
- Modify Collection | ||
- Rename Collection | ||
- Shard Collection | ||
- Drop Collection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing the 2 new sharding types
@nathantfrank : WRT your comment about Deployment only being supported on non-shared tiers, what happens in the UI? Is that box just diabled? What happens if I choose, say, Collection, and then leave Collection Name and DB Name blank (which should force Deploy level)? Thanks! |
Images taken from local branch, not yet merged. So it will be disabled. If they select Deployment and then select a shared tier we will move them automatically to Database level.
If they try to save as described above we have added validation on the backend to respond with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All most there from my perspective just 2 questions.
[Q] Use Match Expressions to Limit Trigger Invocations
only lists the document change event types, do we want to include links for the others?
- :guilabel:`Operation Type`. The :ref:`database operation types | ||
<database-event-operation-types>` that cause the Trigger to fire. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Q] Only this Database row has a link, do we want to have a link for Collection
and Deployment
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we able to time the release or is once this is merged it is live in the documentation?
.. warning:: | ||
|
||
It is possible to configure triggers that cause other triggers to fire on the | ||
same collection, database, or cluster, resulting in recursion. Examples | ||
include a database trigger writing to a collection within the same | ||
database, or a cluster-level logger writing database logs to another database | ||
in the same cluster. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we may want to explicitly call out log forwarders as well here. Since, they are a first class resource that our users may also be using that could cause this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that just a specific case of a process writing to a monitored database and/or collection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct, but just wanted to make customer's slightly more conscious if they didn't make that connection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talked offline in the project channel with team and Laura, we would like to go with.
.. warning:: | |
It is possible to configure triggers that cause other triggers to fire on the | |
same collection, database, or cluster, resulting in recursion. Examples | |
include a database trigger writing to a collection within the same | |
database, or a cluster-level logger writing database logs to another database | |
in the same cluster. | |
.. warning:: | |
In deployment and database level triggers, it is possible to configure triggers | |
in a way that causes other triggers to fire, thereby resulting in recursion. | |
Examples include a database-level trigger writing to a collection within the | |
same database, or a cluster-level logger or log forwarder writing logs to | |
another database in the same cluster. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MongoCaleb Could we get an update on this since it should be going in tomorrow.
@nathantfrank we can delay deployment. When is the anticipated release? |
We are targeting the week after skunkworks, so likely around Nov 1st would be ideal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 Q/potential change, then looks good to me when that is addressed.
- Represents a new document that replaced a document in the collection. | ||
|
||
* - **Create_Collection** (Database and Deployment trigger types only) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Q] Any particular reason we are underscoring here and below?
✨ Staging URL: https://preview-mongodbmongodb.gatsbyjs.io/atlas-app-services/master/ 🪵 Logs |
Pull Request Info
Jira
https://jira.mongodb.org/browse/DOCSP-33588
Staged Changes
https://preview-mongodbmongocaleb.gatsbyjs.io/atlas-app-services/DOCSP-33588/triggers/database-triggers/#std-label-trigger-recursion