[Ingest Manager] Allow prerelease in package version#74452
[Ingest Manager] Allow prerelease in package version#74452jonathan-buttner merged 6 commits intoelastic:masterfrom
Conversation
| const pkgName = pkgkey.substr(0, pkgkey.indexOf('-')); | ||
| // this will return the entire string if `indexOf` return -1 | ||
| const pkgVersion = pkgkey.substr(pkgkey.indexOf('-') + 1); | ||
| return { pkgName, pkgVersion }; |
There was a problem hiding this comment.
I don't think it should accept an empty name or version.
I also think we should ensure the version is valid semver https://github.com/npm/node-semver#usage
There was a problem hiding this comment.
Thanks for the comment @jfsiii, how about if I check if the pkgName is an empty string and throw an error?
For the pkgVersion I can do the same thing, semver.valid(pkgVersion) and if not throw an error?
|
Pinging @elastic/ingest-management (Team:Ingest Management) |
|
|
||
| 'x-pack/plugins/maps/server/fonts/**/*', | ||
| // packages for the ingest manager's api integration tests could be valid semver which has dashes | ||
| 'x-pack/test/ingest_manager_api_integration/apis/fixtures/test_packages/**/*', |
There was a problem hiding this comment.
@jfsiii @skh @neptunian @nchaulet this is so we can have a valid semver as the package directory name with a dash in it. Should this only cover the packages directory? Or would it be useful if it was higher up like:
'x-pack/test/ingest_manager_api_integration/apis/fixtures/**/* ?
There was a problem hiding this comment.
let's start here and lift it up later if we need to
tylersmalley
left a comment
There was a problem hiding this comment.
Pre-commit hook changes look good.
jfsiii
left a comment
There was a problem hiding this comment.
Thanks for adding the function & tests
|
|
||
| 'x-pack/plugins/maps/server/fonts/**/*', | ||
| // packages for the ingest manager's api integration tests could be valid semver which has dashes | ||
| 'x-pack/test/ingest_manager_api_integration/apis/fixtures/test_packages/**/*', |
There was a problem hiding this comment.
let's start here and lift it up later if we need to
| after(async () => { | ||
| if (server.enabled) { | ||
| // remove the package just in case it being installed will affect other tests | ||
| await deletePackage(supertest, pkgkey); |
There was a problem hiding this comment.
I'd prefer to call the delete API here rather than coupling the tests to one another, but it's not a blocker
There was a problem hiding this comment.
Ah ok, I'll switch it back 👍
💚 Build SucceededBuild metrics
History
To update your PR or re-run it, just comment with: |
* Allow prerelease in version * Adding integration test for prerelease version of a package * Tests for invalid package key * Removing inter-test dependency
…-task * master: (42 commits) Allow any hostname for chromium proxy bypass (elastic#74693) [ML] ML on Kibana Management: Add ability to pass a group ID filter to job management page (elastic#74533) [Metrics UI] Fix No Data preview pluralization (elastic#74399) [Bug][Security_Solution][Telemetry] Capitalize S in macOS (elastic#74688) Remove karma tests from legacy maps (elastic#74668) [Ingest Manager] stop creating events-* index pattern and placeholder index (elastic#74683) [Enterprise Search] Update the browser/document title on plugin navigation (elastic#74392) [visualizations] Add i18n translation for 'No results found' (elastic#74619) [maps] convert vector style properties to TS (elastic#74553) bump geckodriver binary to 0.27 (elastic#74638) fix: update apm agents to catch abort requests (elastic#74658) [Security Solution] Resolver children pagination (elastic#74603) add memoryStatus to df analytics page and analytics table in management (elastic#74570) [Ingest Manager] Allow prerelease in package version (elastic#74452) [App Arch]: remove legacy karma tests (elastic#74599) [i18n] revert reverted changes (elastic#74633) [Lens] Clear out all attribute properties before updating (elastic#74483) [Uptime] Fix full reloads while navigating to alert/ml (elastic#73796) Index pattern field class refactor (elastic#73180) [ML] Functional tests - stabilize DFA job type check (elastic#74631) ...
* master: (339 commits) [Ingest Node Pipelines] Sentence-case processor names (elastic#74645) Bump angular dependency from 1.7.9 to 1.8.0 (elastic#74482) [ML] Fixing schema for custom rule conditions (elastic#74676) [ML] Refactor in preparation for new es client (elastic#74552) [ML] Adding initial file analysis overrides (elastic#74376) Allow any hostname for chromium proxy bypass (elastic#74693) [ML] ML on Kibana Management: Add ability to pass a group ID filter to job management page (elastic#74533) [Metrics UI] Fix No Data preview pluralization (elastic#74399) [Bug][Security_Solution][Telemetry] Capitalize S in macOS (elastic#74688) Remove karma tests from legacy maps (elastic#74668) [Ingest Manager] stop creating events-* index pattern and placeholder index (elastic#74683) [Enterprise Search] Update the browser/document title on plugin navigation (elastic#74392) [visualizations] Add i18n translation for 'No results found' (elastic#74619) [maps] convert vector style properties to TS (elastic#74553) bump geckodriver binary to 0.27 (elastic#74638) fix: update apm agents to catch abort requests (elastic#74658) [Security Solution] Resolver children pagination (elastic#74603) add memoryStatus to df analytics page and analytics table in management (elastic#74570) [Ingest Manager] Allow prerelease in package version (elastic#74452) [App Arch]: remove legacy karma tests (elastic#74599) ...
This PR switches the parsing of package keys (e.g.
endpoint-0.13.0) to not usesplit()but instead look for the first occurrence of the dash delimiter.Working prerelease
Package key failures