Skip to content
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

Ship packages as zip instead of tar.gz #628

Merged
merged 2 commits into from
Sep 8, 2020
Merged

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Aug 27, 2020

This is a breaking change and first needs changes on the Kibana side. The new registry and the new version of Kibana should be either rolled out in sync or Kibana supports both for a bit.

Closes #474

@ruflin ruflin self-assigned this Aug 27, 2020
@elasticmachine
Copy link

elasticmachine commented Aug 27, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #628 updated]

  • Start Time: 2020-09-08T07:23:13.343+0000

  • Duration: 8 min 25 sec

Test stats 🧪

Test Results
Failed 0
Passed 161
Skipped 0
Total 161

@ruflin ruflin marked this pull request as ready for review August 27, 2020 11:58
@ruflin
Copy link
Contributor Author

ruflin commented Aug 27, 2020

Opening this as ready for review even though CI will be red on the integration tests. The reason is that Kibana does not support zip at the moment.

@ruflin ruflin changed the title Switch to zip Ship packages as zip instead of tar.gz Aug 27, 2020
@mtojek
Copy link
Contributor

mtojek commented Aug 27, 2020

Do you think it's feasible to prepare an archiver abstraction and support two modes (zip, tar.gz)?

@ruflin
Copy link
Contributor Author

ruflin commented Aug 27, 2020

@mtojek Definitively possible but what would be the advantage of having the abstraction if we only plan to support one?

@mtojek
Copy link
Contributor

mtojek commented Aug 27, 2020

Last time we also planned to support single one :) In the description you've written that we can't push it until Kibana supports the zip format. With an appropriate abstraction we don't need to wait for this.

@ruflin
Copy link
Contributor Author

ruflin commented Aug 31, 2020

I realised, only Kibana can handle both as it is a single entry in the registry output (download). Lets not add abstraction for it before we are sure we will need it in the future.

As soon as elastic/kibana#76197 is merged I'll trigger CI again to see if it goes green.

@mtojek
Copy link
Contributor

mtojek commented Aug 31, 2020

The implementation looks good to me, let's wait for the CI to pass all tests.

@ruflin
Copy link
Contributor Author

ruflin commented Sep 2, 2020

jenkins, test this

@ruflin
Copy link
Contributor Author

ruflin commented Sep 7, 2020

With 7.10 docker images from Kibana updated, this now seems to pass CI too and should be ready to be merged. @jfsiii @mtojek Would be good to get from both of you the 👍

@mtojek mtojek self-requested a review September 7, 2020 12:09
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

single nit-pick only

archiver/archive.go Outdated Show resolved Hide resolved
This is a breaking change and first needs changes on the Kibana side. The new registry and the new version of Kibana should be either rolled out in sync or Kibana supports both for a bit.

Closes elastic#474
@ruflin ruflin merged commit cfd9586 into elastic:master Sep 8, 2020
@ruflin ruflin deleted the switch-to-zip branch September 8, 2020 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discuss: Move to zip instead of tar.gz for the format of the package.
3 participants