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.

Sign up for GitHub

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

Jump to bottom

No longer package X-Pack as a node module #32722

Merged
merged 5 commits into from
Apr 10, 2019
Merged

No longer package X-Pack as a node module #32722

merged 5 commits into from
Apr 10, 2019

Conversation

tylersmalley
Copy link
Contributor

@tylersmalley tylersmalley commented Mar 7, 2019 •
edited
Loading

Fixes #31471

  • Only builds X-Pack for non-OSS builds which accounted for ~1m30s in those functional tests.

@tylersmalley tylersmalley added Team:Operations Team label for Operations Team v8.0.0 v7.2.0 labels Mar 7, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations

@tylersmalley tylersmalley mentioned this pull request Mar 7, 2019
Closed
@elastic elastic deleted a comment from elasticmachine Mar 8, 2019
tylersmalley
tylersmalley commented Mar 8, 2019
View reviewed changes
x-pack/tasks/build.js Outdated
const pkg = JSON.parse(readFileSync(resolve(buildRoot, 'package.json')));

pkg.dependencies = Object.keys(pkg.dependencies)
.filter((d) => !projects.has(d))
Copy link
Contributor Author

@tylersmalley tylersmalley Mar 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I am thinking we could additionally remove any dependencies with the same version qualifier as the dependency in Kibana. Thoughts? I still need to see how much additional overhead we are looking at currently.

Copy link
Member

Choose a reason for hiding this comment

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

can you add any context on what's happening? wouldn't yarn workspace dedupe these to the top level automatically?

Copy link
Member

Choose a reason for hiding this comment

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

oh, this is outside the scope of kbn-pm et all as a side effect

@tylersmalley tylersmalley marked this pull request as ready for review March 8, 2019 00:56
@tylersmalley tylersmalley requested a review from a team as a code owner March 8, 2019 00:56
@elasticmachine

This comment has been minimized.

Sign in to view
@elasticmachine

This comment has been minimized.

Sign in to view
@elasticmachine

This comment has been minimized.

Sign in to view
@jasonrhodes
Copy link
Member

@tylersmalley so just for our own clarification's sake in APM ... the imports that used 'x-pack/plugins/etc' paths only worked because x-pack was packaged as a node module? That little "trick" only worked for us in .ts(x) files so I always assumed it was TS compiler magic, even though I could never quite track down how it worked...

jasonrhodes
jasonrhodes approved these changes Mar 14, 2019
View reviewed changes
Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

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

LGTM fror APM ...

Sidenote: would love to consider some way of aliasing imports from Kibana's root to avoid relative imports that go 3+ levels, much easier to read:
from 'kibana/x-pack/plugins/apm/typings/Transaction'
than
from '../../../../../../../../../../typings/Transaction'

@elasticmachine

This comment has been minimized.

Sign in to view
@elasticmachine

This comment has been minimized.

Sign in to view
@elasticmachine

This comment has been minimized.

Sign in to view
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers lukeelmers mentioned this pull request Mar 22, 2019
Merged
@elasticmachine

This comment has been minimized.

Sign in to view
@elasticmachine

This comment has been minimized.

Sign in to view
@tylersmalley
Copy link
Contributor Author

@jbudz mind reviewing this?

@tylersmalley
Copy link
Contributor Author

@spalger might have some opinions as well.

@jbudz
Copy link
Member

jbudz commented Mar 25, 2019 •
edited
Loading

It looks like x-pack is still in node_modules, (and in root).

@elasticmachine

This comment has been minimized.

Sign in to view
@elasticmachine

This comment has been minimized.

Sign in to view
No longer package X-Pack as a node module
a039578 
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@elasticmachine

This comment has been minimized.

Sign in to view
jbudz
jbudz reviewed Apr 4, 2019
View reviewed changes
Copy link
Member

@jbudz jbudz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It looks like we're missing -oss on the extracted folder?

➜  Downloads ls
kibana-oss-8.0.0-SNAPSHOT-linux-x86_64.tar.gz
➜  Downloads tar -xf kibana-oss-8.0.0-SNAPSHOT-linux-x86_64.tar.gz >/dev/null
➜  Downloads ls
kibana-8.0.0-SNAPSHOT-linux-x86_64            kibana-oss-8.0.0-SNAPSHOT-linux-x86_64.tar.gz

edit: nevermind, that's consistent with 6.7

@elasticmachine

This comment has been minimized.

Sign in to view
@jbudz
Copy link
Member

jbudz commented Apr 4, 2019

I'm getting
image locally with the x-pack build. oss is fine. maybe consistent with the CI failures
image

@tylersmalley
Copy link
Contributor Author

Regarding the -oss on the extracted folder - there was a stack decision when we opened x-pack to keep it the same as it would have been more of a breaking change for a minor version than we were comfortable with.

@tylersmalley tylersmalley mentioned this pull request Apr 5, 2019
Closed
@elasticmachine

This comment has been minimized.

Sign in to view
Resolve symlinks and clean packages dir
af23c65 
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley
Copy link
Contributor Author

tylersmalley commented Apr 7, 2019 •
edited
Loading 

16:11:05      │ info verifying cache of https://snapshots.elastic.co/8.0.0-55bab4fb/downloads/elasticsearch/elasticsearch-8.0.0-SNAPSHOT-linux-x86_64.tar.gz
16:12:07 
16:12:07 Unable to download elasticsearch snapshot: Service Unavailable
16:12:07   accept-ranges: bytes
16:12:07   connection: close
16:12:07   content-length: 108
16:12:07   content-type: text/html
16:12:07   date: Sun, 07 Apr 2019 23:12:07 GMT
16:12:07   retry-after: 0
16:12:07   server: ElasticInfrastructure
16:12:07   via: 1.1 varnish
16:12:07   x-cache: MISS
16:12:07   x-cache-hits: 0
16:12:07   x-served-by: cache-mdw17347-MDW
16:12:07   x-timer: S1554678666.594070,VS0,VE61543
16:12:07   accept-ranges: bytes
16:12:07   connection: close
16:12:07   content-length: 108
16:12:07   content-type: text/html
16:12:07   date: Sun, 07 Apr 2019 23:12:07 GMT
16:12:07   retry-after: 0
16:12:07   server: ElasticInfrastructure
16:12:07   via: 1.1 varnish
16:12:07   x-cache: MISS
16:12:07   x-cache-hits: 0
16:12:07   x-served-by: cache-mdw17347-MDW
16:12:07   x-timer: S1554678666.594070,VS0,VE61543
16:12:07     at Artifact._download (/var/lib/jenkins/workspace/elastic+kibana+pull-request/JOB/x-pack-ciGroup7/node/immutable/kibana/packages/kbn-es/src/artifact.js:221:13)
16:12:07     at process._tickCallback (internal/process/next_tick.js:68:7)

retest

@elasticmachine

This comment has been minimized.

Sign in to view
Update snapshot
0a51436 
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@sorenlouv sorenlouv mentioned this pull request Apr 9, 2019
Closed
@tylersmalley tylersmalley merged commit b9f81ed into elastic:master Apr 10, 2019
@tylersmalley tylersmalley mentioned this pull request Apr 10, 2019
Merged
tylersmalley added a commit to tylersmalley/kibana that referenced this pull request Apr 10, 2019
@tylersmalley
No longer package X-Pack as a node module (elastic#32722)
fe9e441 
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
tylersmalley added a commit that referenced this pull request Apr 10, 2019
@tylersmalley
No longer package X-Pack as a node module (#32722) (#34886)
ca20278 
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley tylersmalley added the non-issue Indicates to automation that a pull request should not appear in the release notes label Apr 10, 2019
chandlerprall pushed a commit to chandlerprall/kibana that referenced this pull request Apr 15, 2019
@tylersmalley @chandlerprall
No longer package X-Pack as a node module (elastic#32722)
9f3e704 
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@cjcenizal cjcenizal mentioned this pull request Apr 18, 2019
Merged
@mshustov mshustov mentioned this pull request Apr 30, 2019
Merged
11 tasks
@lukeelmers lukeelmers mentioned this pull request Nov 19, 2019
Merged
3 tasks
@ankane ankane mentioned this pull request Jun 9, 2020
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spalger spalger spalger left review comments

@mistic mistic mistic left review comments

@jasonrhodes jasonrhodes jasonrhodes approved these changes

@jbudz jbudz jbudz approved these changes

Assignees
No one assigned
Labels
non-issue Indicates to automation that a pull request should not appear in the release notes Team:Operations Team label for Operations Team v7.2.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tylersmalley @elasticmachine @jasonrhodes @jbudz @spalger @mistic