Skip to content

Ship packages as zip instead of tar.gz#628

Merged
ruflin merged 2 commits intoelastic:masterfrom
ruflin:switch-to-zip
Sep 8, 2020
Merged

Ship packages as zip instead of tar.gz#628
ruflin merged 2 commits intoelastic:masterfrom
ruflin:switch-to-zip

Conversation

@ruflin
Copy link
Copy Markdown
Collaborator

@ruflin ruflin commented Aug 27, 2020 •
edited
Loading

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
Copy Markdown

elasticmachine commented Aug 27, 2020 •
edited
Loading

💚 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
Copy Markdown
Collaborator 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
@ruflin ruflin mentioned this pull request Aug 27, 2020
Closed
@mtojek
Copy link
Copy Markdown
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
Copy Markdown
Collaborator 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
Copy Markdown
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.

@jfsiii jfsiii mentioned this pull request Aug 27, 2020
Merged
1 task
@ruflin
Copy link
Copy Markdown
Collaborator 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
Copy Markdown
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
Copy Markdown
Collaborator Author

ruflin commented Sep 2, 2020

jenkins, test this

@ruflin
Copy link
Copy Markdown
Collaborator 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
mtojek
mtojek approved these changes Sep 7, 2020
View reviewed changes
Copy link
Copy Markdown
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

@ruflin
Ship packages as zip instead of tar.gz
5ef5357 
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
update error message
286be87
@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

Reviewers

1 more reviewer

@mtojek mtojek mtojek approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ruflin ruflin

Labels

None yet

Projects

None yet

Milestone

No milestone

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

@ruflin @elasticmachine @mtojek