Remove _meta/kibana.generated symlink #9892
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cwurm
added
review
needs_backport
|
@cwurm I was also looking at this and came to a similar solution (see dashboards.patch). But with few differences:
Do you want to incorporate some of this with your PR? |
|
@andrewkroh Thanks, definitely! I've pushed a new commit. |
kvch
approved these changes
Jan 7, 2019
kvch
added
v6.7.0
and removed
needs_backport
kvch
pushed a commit
to kvch/beats
that referenced
this pull request
Jan 7, 2019
Fixes a bug introduced with elastic#9546 where a symlink from `_meta/kibana.generated` to `build/kibana` would cause objects to be included in the dashboard ZIP file with the wrong path. This removes the symlink in favor of a conditional. Fixes elastic#9785.
kvch
pushed a commit
to kvch/beats
that referenced
this pull request
Jan 7, 2019
Fixes a bug introduced with elastic#9546 where a symlink from `_meta/kibana.generated` to `build/kibana` would cause objects to be included in the dashboard ZIP file with the wrong path. This removes the symlink in favor of a conditional. Fixes elastic#9785. (cherry picked from commit 3a51720)
|
@cwurm This should not have been merged. It seems this change caused the failure for the test_ml jobs. The error is: I assume this will also affect the backports? |
This was referenced Jan 8, 2019
Merged
ruflin
added a commit
to ruflin/beats
that referenced
this pull request
Jan 8, 2019
In elastic#9892 the build was broken by removing the symlink to the generated kibana files which broke the ml tests. This fixes the test by pointing to the new directory under build/kibana. Closes elastic#9938
ruflin
added a commit
that referenced
this pull request
Jan 8, 2019
|
@ruflin Ah you're right, sorry. Thanks for fixing it in master.
It shouldn't. In 6.x and 6.6 Filebeat is still using CI for 6.x and 6.6 look good for Filebeat. |
leweafan
pushed a commit
to leweafan/beats
that referenced
this pull request
Apr 28, 2023
Fixes a bug introduced with elastic#9546 where a symlink from `_meta/kibana.generated` to `build/kibana` would cause objects to be included in the dashboard ZIP file with the wrong path. This removes the symlink in favor of a conditional. Fixes elastic#9785. (cherry picked from commit 23f1ea2)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#9546 introduced a symlink from
_meta/kibana.generatedtobuild/kibanaso Auditbeat remains backwards compatible with the other Beats (Auditbeat is already using the first as a location for all its Kibana objects, while the other Beats are still using the latter).This introduced a bug, where objects would be included in the dashboard ZIP file with the wrong path (#9785).
One way of fixing this would have been to double down on symlinks and change just one line in
addFileToZip. But with #9842 about to change all Beats to use the newbuild/kibanaI thought it better to just remove the symlink capability altogether and instead if-else between the two possible locations based on whether they exist or not. #9842 will then change all of that - at least inmaster- once it's merged.Fixes #9785.