Create multiple files for monitors rather than one big monitors file + Go Modules for dep management #13
Conversation
| monitorFile := filepath.Join(monitorsPath, sanitize.BaseName(name) + ".yaml") | ||
| file, err := os.OpenFile(monitorFile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) | ||
| check(err) | ||
| data, err := yaml.Marshal(monitors[name]) | ||
| check(err) | ||
| file.Write(data) |
There was a problem hiding this comment.
So we'll be creating new file for each new monitor ?
There was a problem hiding this comment.
Yep that's right. We have about 30-40 monitors per env and they are pretty lengthy, so it made sense to keep them separated out.
There was a problem hiding this comment.
So initial my thoughts on sync is for just to start with all existing monotirs form cluster and organize however users would want.
There was a problem hiding this comment.
Meaning let users choose whether they want one big yaml file vs a yaml file per monitor?
| @@ -48,22 +48,21 @@ func GetAllLocal(rootDir string) (map[string]Monitor, mapset.Set, error) { | |||
| monitorsSet := mapset.NewSet() | |||
| monitorsMap := make(map[string]Monitor) | |||
| for _, file := range files { | |||
| var allLocalMonitors []Monitor | |||
| var monitor Monitor | |||
There was a problem hiding this comment.
This will fail for users who has multiple monitors in a single YAML file right ?
There was a problem hiding this comment.
I believe so, this would be a breaking change.
There was a problem hiding this comment.
I am just afraid of this change, do we really need this ? Also, this will make users use case. There are users who might categorize monitors and then reduce files. If there are too many monitors it would make hard to manage 100s of YML files.
There was a problem hiding this comment.
So personally, I think 100s of yaml files is better than one huge yaml file. Most of our monitors are between 80-120 LOC. If we had 100s, that would equate to a yaml file of about 10k lines.
Lots of yaml backed programs like kubernetes usually use individual yaml files to configure their applications.
I'm not certain what you mean by this:
There are users who might categorize monitors and then reduce files.
There was a problem hiding this comment.
There are users who might categorize monitors and then reduce files.
Basically, let's say you've 100 monitors and but one would categorized file as product or feature and have multiple monitors in same file. As I mentioned earlier,sync is to just a utility to get started from existing monitors and then you can still choose to have all monitors in different files.
IMO, I would rather let user choose how they would want to organize these monitors instead of not prefer to enforce this by default but?
Thoughts ?
There was a problem hiding this comment.
I think most users will want to sync before making changes since people might have made a change in the UI and the monitors yaml will be out of date. If you reorganize the output of the monitors from sync, then sync will not update those monitors. That's our workflow, we sync nightly and before making any changes to make sure all changes from the UI get populated.
There was a problem hiding this comment.
I think enforcing one's workflow to everyone is not great way. I think it would be best for users to come up with their own usecase and workflow.
In order to solve the use case you described , how about we change sync in a way that it goes through all YMLs and then if found it will update the same object and if not it will in last dump all new monitors which are being synced for the first time to the new file and then one can organize and later it the state will be persisted. This way it user can decided and sync correctly.
In a nutshell, following
- first time sync , dump everything into new file
- User organize their monitors
- user modify / updates
- User sync again, in this scenario
- verify first if this monitor exists, if it does ensure we update the same file
- Find a delta and leftovers (i.e new monitors) will be dumped to new file and then one can organize in their comfortable use case.
My idea here is to not user to enforce a way rather just mechanism.
There was a problem hiding this comment.
I like your sync change and I'm on board.
One question: are you still opposed to keeping a yaml file per monitor? To solve the category organization problem users can organize the monitors by folder.
There was a problem hiding this comment.
I am not opposed, I feel it shouldn't be enforced by tool. I would let users choose their flow, they can categorize per file or per features up-to them.
|
Any news on this matter ? We would also be interested in many smaller files rather than a big one with every monitors. |
As it stands unfortunately I don’t have time to implement backwards compatibility in this PR like the author wants. I plan on getting to it... sometime in the future. |
Ah, what a shame. Thank you for your PR nonetheless, we'll also use it to implement the same feature on our side. |
This is a breaking change from the current implementation. One big monitors yaml will not work anymore.
Sorry about the reverts. I assume it'll be squash and merged anyway.