add hostname to backup manifest#18847
Conversation
Signed-off-by: --global <rrangel@slack-corp.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
| if hostname, err := os.Hostname(); err == nil { | ||
| require.Equal(t, hostname, manifest.Hostname) | ||
| } | ||
|
|
There was a problem hiding this comment.
the mysqlshell engine is the only one we have unit tests for ExecuteBackup, so I only verified this here
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #18847 +/- ##
=======================================
Coverage 69.70% 69.70%
=======================================
Files 1607 1607
Lines 214579 214620 +41
=======================================
+ Hits 149572 149605 +33
- Misses 65007 65015 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
timvaillancourt
left a comment
There was a problem hiding this comment.
Makes sense to me 👍
| }() | ||
|
|
||
| // Get the hostname | ||
| hostname, err := os.Hostname() |
There was a problem hiding this comment.
We also have netutil.FullyQualifiedHostname, should that be used instead?
There was a problem hiding this comment.
ah cool, I didn't know we had that, I have updated the PR
Signed-off-by: --global <rrangel@slack-corp.com>
Signed-off-by: Renan Rangel <rrangel@slack-corp.com>
Signed-off-by: Renan Rangel <rrangel@slack-corp.com>
Signed-off-by: Renan Rangel <rrangel@slack-corp.com>
Signed-off-by: Renan Rangel <rrangel@slack-corp.com>
Signed-off-by: Renan Rangel <rrangel@slack-corp.com>
Signed-off-by: Renan Rangel <rrangel@slack-corp.com>
Description
This PR adds the hostname of the host taking the backup to the
BackupManifestto make it easier to identify and filter backups with that information as well.Related Issue(s)
BackupManifest#18846Checklist
Deployment Notes
AI Disclosure