Bump actions/upload-artifact from 2 to 3#2
Open
dependabot[bot] wants to merge 1 commit into
Open
Conversation
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 2 to 3. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v2...v3) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
wirybeaver
pushed a commit
that referenced
this pull request
Feb 28, 2025
wirybeaver
pushed a commit
that referenced
this pull request
May 9, 2025
* Addition of initial spi change checker code * Fixes to yaml file, mainly excluding artifacts.zip code to test run further * changing shell file permissions * add "exit 1" to shell script and mess with TableConfig method signature * changing shell file permissions again for some reason * changing git diff checker file path * second git diff checker file path change * removing unnecessary code from files, fixed issue with running GitDiffChecker, added functionality for displaying line number, and reverted temporary change to TableConfig.java * permission changes * changing permissions * fixing compilation error * fixing parameterization of commits * trial and error yml file #1 * commit for testing that config file correctly has parameters, changing sh file so that "No incorrect..." message only displayed once, and adding blank line check to GitDiffChecker * GitDiffChecker: add case to skip ---. otherwise, change parameters to work with pull requests. * testing * testing #2 * Revert "testing #2" This reverts commit 270937f. * yml: change main to master. shell: change main to master and change conditional to reflect return type change. java: switch from returning line number to string of code, as previous logic did not work if multiple "chunks" of code were changed, and added annotation logic that excluded json-related annotations. * yml: comment out apache/pinot condition for now. shell: change error message and put text file in my module. java: slightly altered regex after rethinking it, removed System.out.println, i think it's not necessary * java: slightly altered regex to account for interface definitions using semicolons and not curly braces * changing "no incorrect spi changes" value to "0", so if the method returns nothing, there isn't an accidental test passing * minor change to annotation regex, \n changed to $ (end of string) * fix to pom.xml * Added logic for outputting line number along with original file code snippet * per testing on another branch, slightly updating line number logic * Removed my outdated custom Java implementation of pinot-spi change checking. Switched to the japicmp plugin, with some modifications to the compatibility of checks that japicmp performs. With this, contributors will be able to see if they made incompatible SPI changes when running mvn clean, rather than waiting until they make a PR. Adding a .jar of pinot-spi that japicmp will use for comparisons. * Per Tianle's comment, added a comment in the pom file explaining our justification for using a baseline jar for comparison, and that we need to eventually transition away from it. * Removed code that made annotation changes/deletions incompatible. Fixed pom file so that all pinot-spi files are checked * updated baseline .jar to match updates from apache:master
wirybeaver
pushed a commit
that referenced
this pull request
Jun 14, 2026
…tionIdNormalizer (apache#18446) * Make PartitionFunction registry dynamic; move impls to pinot-common Replaces the closed `PartitionFunctionFactory` enum/switch with an annotation-based registry so plug-in partition functions no longer require touching segment-spi code. - Add `@PartitionFunctionType(names = {...})` annotation in pinot-spi. - Convert `PartitionFunctionFactory` to a classpath-scanning registry (regex `.*\.partition\.function\..*`); the factory keeps the same static API, callers are unchanged. Add `init()` mirroring `FunctionRegistry.init()` and wire it from broker / server / controller starters. - Move the seven built-in impls (`Modulo`, `Murmur` / `Murmur2`, `Murmur3`, `Fnv`, `HashCode`, `ByteArray`, `BoundedColumnValue`) out of `pinot-segment-spi` into `pinot-common/.../partition/function/`, standardize their constructor on `(int numPartitions, Map<String,String> functionConfig)`, and annotate each. - Add `PartitionIntNormalizer` enum (`POSITIVE_MODULO` / `ABS` / `MASK`) in pinot-segment-spi and a default `getPartitionIdNormalizer()` on `PartitionFunction` so impls can declare which normalizer matches their internal modulo semantics. Used by the framework only for identity / staleness matching between config-side and segment-side metadata; legacy impls still compute their own modulo. Javadoc spells out the descriptive-only nature of the value for legacy functions. - Tests: existing `PartitionFunctionTest` (19 cases) moved to pinot-common; new `PartitionFunctionFactoryTest` covers registration completeness, alias resolution (`Murmur` / `Murmur2`), case-insensitive lookup, idempotent `init()`, unknown-name rejection, and per-impl normalizer label; new `PartitionIntNormalizerTest` covers per-normalizer math at edge cases (`Integer.MIN_VALUE`, `Integer.MAX_VALUE`), range invariant across all normalizers, and `fromConfigString` round-trip / blank / unknown. Plug-in path going forward: drop a class on the classpath under `*.partition.function.*`, implement `PartitionFunction` with the standard ctor, add `@PartitionFunctionType(names = "MyFn")` - the registry picks it up at startup. NOTE - backward-incompat: the seven impl classes' fully-qualified names changed (`org.apache.pinot.segment.spi.partition.*` -> `org.apache.pinot.common.partition.function.*`) and the no-Map constructors on `Modulo` / `HashCode` / `ByteArray` are gone in favor of the standard `(int, Map<String,String>)` form. `getName()` strings (`Modulo`, `Murmur`, ...) are unchanged, so segment-on-disk metadata is unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Drop FnvPartitionFunction inner enum; use PartitionIntNormalizer directly Removes the duplicated `NegativePartitionHandling` inner enum in `FnvPartitionFunction` and the `_negativePartitionHandling` field in favor of a single `_normalizer` of type `PartitionIntNormalizer`. The config key (`negativePartitionHandling`) and accepted values stay unchanged for `mask` / `abs`; `positive_modulo` is now also acceptable (strict superset). The error message for unknown values now comes from `PartitionIntNormalizer.fromConfigString`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Make getPartitionIdNormalizer() non-null on PartitionFunction The interface default now returns `PartitionIntNormalizer.POSITIVE_MODULO` instead of null, and `@Nullable` is dropped from the method. Plug-ins that don't map onto a standard normalizer (e.g. `BoundedColumnValue`, which already produces ids in `[0, N)`) inherit the safe default without needing an explicit override. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Make getPartitionIdNormalizer() abstract; each impl declares its own Removes the interface default so every PartitionFunction must explicitly pick a normalizer. BoundedColumnValuePartitionFunction now overrides with POSITIVE_MODULO (no-op label, since its output is already a fixed mapping in [0, N)). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Drive each partition function's int-to-id mapping via _normalizer field Every built-in PartitionFunction now stores a `_normalizer` of type PartitionIntNormalizer and applies it in `getPartition(...)` rather than open-coding the modulo logic. Defaults match prior behavior: | Impl | Default normalizer | |-------------------|--------------------| | Modulo | POSITIVE_MODULO | | Murmur / Murmur2 | MASK | | Murmur3 | MASK | | Fnv | MASK | | HashCode | KAFKA_ABS (new) | | ByteArray | KAFKA_ABS (new) | | BoundedColumnValue| POSITIVE_MODULO | Adds `KAFKA_ABS` to PartitionIntNormalizer to cover the Kafka-style `abs(hash) % N` (with `Integer.MIN_VALUE -> 0`) used by HashCode and ByteArray. With KAFKA_ABS in place the normalizer is now an authoritative driver of the partition-id computation, not just a descriptive label - the interface Javadoc is updated accordingly. Unifies the per-impl override path under a single config key `partitionIdNormalizer` (case-insensitive), parsed via shared `PartitionFunctionConfigs#normalizer`. Drops the FNV-specific `negativePartitionHandling` key. Tests: - New `KAFKA_ABS` cases in PartitionIntNormalizerTest covering the MIN_VALUE corner. - New `testPartitionIdNormalizerConfigOverridesDefaultAcrossImpls` in PartitionFunctionFactoryTest verifying the config rewires the computed partition for HashCode, Modulo, and ByteArray. - FNV tests updated to use `partitionIdNormalizer` config key. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Convert new Javadoc to JEP 467 markdown style Switches all new Javadoc blocks added by this PR from `/** */` block syntax to `///` markdown syntax (JEP 467), matching the convention established in apache#18165 and the rest of the recently-touched pinot-segment-spi files. Replaces `<p>`, `<ul>/<li>`, `<code>`, `{@code X}`, `{@link X}` HTML/Javadoc tags with their markdown equivalents (paragraph breaks, `-` lists, backticks, `[X]` refs). License headers remain as `/** */` block comments. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address PR review feedback - Rename `PartitionIntNormalizer` -> `PartitionIdNormalizer` (Jackie #3). - Rename `KAFKA_ABS` -> `PRE_MODULO_ABS` to make the pre-vs-post-modulo distinction with `ABS` explicit (Jackie #4). - Change `getPartitionIdNormalizer()` to return `PartitionIdNormalizer` enum directly instead of `String`; drop `@JsonIgnore` so the field is visible in serialized form (Jackie #2). Method stays abstract -- every PartitionFunction implementation declares its own normalizer. - Drop the redundant `_normalizer` field from `BoundedColumnValuePartitionFunction`; it now returns `PartitionIdNormalizer.POSITIVE_MODULO` directly since its output is already a fixed mapping in `[0, numPartitions)` (Jackie #6). - Make `@PartitionFunctionType` annotation optional. The factory now scans every public, concrete `PartitionFunction` subtype under the `org.apache.pinot.*` package tree. When a class lacks the annotation (or `names()` is empty), the registry probes `PartitionFunction.getName()` by instantiating with `(1, null)` and registers under the returned name (Jackie #5). Annotation is now a pure aliasing / overriding mechanism. - Fix Javadoc on `@PartitionFunctionType` and `PartitionFunctionFactory` to drop the incorrect "any plugin package" claim and the unimplemented "stripping underscores" note (Copilot #1, #2, #4). - Add `UnannotatedTestPartitionFunction` fixture + factory test that exercises the no-annotation `getName()` fallback path. - Update `PartitionFunctionTest` to expect 4 JSON fields (the new `partitionIdNormalizer` field is no longer hidden). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Trim and reject blank @PartitionFunctionType names; fall back to getName() Validates each entry in `@PartitionFunctionType.names()` at registry build time: - Each entry is trimmed of surrounding whitespace. - Blank entries are dropped silently. - When ALL declared entries are blank (or the array is empty after filtering), the registry logs a warning and falls back to probing `PartitionFunction.getName()`, the same path used for unannotated classes. This prevents a misconfigured annotation (e.g. `names = {" "}`) from silently registering a function under an empty canonical name, making it undiscoverable at lookup time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Drop @PartitionFunctionType annotation; drive registry from getNames() default The annotation added no value beyond what `PartitionFunction.getName()` already returned for every built-in. Remove it entirely and let the interface itself declare the registry contract: - New default `List<String> getNames()` on `PartitionFunction` returns `[getName()]`. The factory's static scan instantiates each subtype with `(1, null)` and registers under whatever `getNames()` returns. - Only `MurmurPartitionFunction` overrides `getNames()` (returns `["Murmur", "Murmur2"]` so both aliases resolve to the same impl); the other six built-ins use the default. - `getNames()` is `@JsonIgnore`'d so it doesn't pollute the serialized form (`testBasicProperties` JSON-shape assertion stays at four fields). - `BoundedColumnValuePartitionFunction`'s ctor now tolerates `null` config (probe path) and defers validation; real-config use still throws as before. `getPartition()` rejects probe-built instances. - `@PartitionFunctionType` annotation file deleted. - `UnannotatedTestPartitionFunction` fixture removed (no longer meaningful now that every class is "unannotated"). - Added two new factory tests covering the default `getNames()` behavior and Murmur's override. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add NO_OP PartitionIdNormalizer for already-in-range outputs `BoundedColumnValuePartitionFunction` produces a fixed mapping in `[0, numPartitions)` by construction; the previous label was `POSITIVE_MODULO` (a no-op for in-range values, but semantically overloaded). Add an explicit `NO_OP` value that is the identity on its inputs and use it for `BoundedColumnValue`. The framework does not validate that callers actually pass in-range values to NO_OP — out-of-range inputs yield out-of-range partition ids, by design. `PartitionIdNormalizerTest#testRangeFoldingNormalizersReturnInRange` (was `testAllNormalizersReturnInRange`) now skips NO_OP since the range invariant doesn't apply. New `testNoOpIsIdentity` locks the identity-with-narrowing semantics including the explicit out-of-range pass-through. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or 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
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.
Bumps actions/upload-artifact from 2 to 3.
Release notes
Sourced from actions/upload-artifact's releases.
... (truncated)
Commits
3cea537Merge pull request #327 from actions/robherley/artifact-1.1.0849aa77nvm use 12 & npm run release4d39869recompile with correct ncc version2e0d362bump@actions/artifactto 1.1.009a5d6aMerge pull request #320 from actions/dependabot/npm_and_yarn/ansi-regex-4.1.1189315dBump ansi-regex from 4.1.0 to 4.1.1d159c2dMerge pull request #297 from actions/dependabot/npm_and_yarn/ajv-6.12.6c26a7baBump ajv from 6.11.0 to 6.12.66ed6c72Merge pull request #303 from actions/dependabot/npm_and_yarn/yargs-parser-13.1.22aeee26Bump yargs-parser from 13.1.1 to 13.1.2You can trigger a rebase of this PR by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)