Skip to content

Conversation

@mhl-b
Copy link
Contributor

@mhl-b mhl-b commented Aug 8, 2024

Rename xContent.streamSeparator() and RestHandler.supportsStreamContent()
to xContent.bulkSeparator() and RestHandler.supportsBulkContent().

I want to reserve use of "supportsStreamContent" for current work in HTTP layer
to support incremental content handling besides fully aggregated byte buffers.
supportsStreamContent would indicate that handler can parse chunks of http content
as they arrive.

@mhl-b mhl-b added Team:Core/Infra Meta label for core/infra team >non-issue labels Aug 8, 2024
@mhl-b mhl-b marked this pull request as ready for review August 8, 2024 19:09
@mhl-b mhl-b requested a review from a team as a code owner August 8, 2024 19:09
@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label and removed Team:Core/Infra Meta label for core/infra team labels Aug 8, 2024
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM, one nit

* Indicates if the RestHandler supports content as a stream. A stream would be multiple objects delineated by
* {@link XContent#streamSeparator()}. If a handler returns true this will affect the types of content that can be sent to
* this endpoint.
* Indicates if the RestHandler supports content as a bulk. A bulk would be multiple objects
Copy link
Member

Choose a reason for hiding this comment

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

nit: this phrasing sounds odd (using bulk as a non), I suggest "supports bulk content" and "A bulk request contains multiple objects..."

@rjernst rjernst added :Core/Infra/REST API REST infrastructure and utilities >refactoring and removed >non-issue needs:triage Requires assignment of a team area label labels Aug 8, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Aug 8, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@mhl-b mhl-b added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Aug 8, 2024
@mhl-b mhl-b changed the title Rename streamContent/Separator to bulkContent/separator Rename streamContent/Separator to bulkContent/Separator Aug 8, 2024
@elasticsearchmachine elasticsearchmachine merged commit 1163d2e into elastic:main Aug 8, 2024
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Aug 9, 2024
* upstream/main: (22 commits)
  Prune changelogs after 8.15.0 release
  Bump versions after 8.15.0 release
  EIS integration (elastic#111154)
  Skip LOOKUP/INLINESTATS cases unless on snapshot (elastic#111755)
  Always enforce strict role validation (elastic#111056)
  Mute org.elasticsearch.xpack.esql.analysis.VerifierTests testUnsupportedAndMultiTypedFields elastic#111753
  [ML] Force time shift integration test (elastic#111620)
  ESQL: Add tests for sort, where with unsupported type (elastic#111737)
  [ML] Force time shift documentation (elastic#111668)
  Fix remote cluster credential secure settings reload   (elastic#111535)
  ESQL: Fix for overzealous validation in case of invalid mapped fields (elastic#111475)
  Pass allow security manager flag in gradle test policy setup plugin (elastic#111725)
  Rename streamContent/Separator to bulkContent/Separator (elastic#111716)
  Mute org.elasticsearch.tdigest.ComparisonTests testSparseGaussianDistribution elastic#111721
  Remove 8.14 from branches.json
  Only emit product origin in deprecation log if present (elastic#111683)
  Forward port release notes for v8.15.0 (elastic#111714)
  [ES|QL] Combine Disjunctive CIDRMatch (elastic#111501)
  ESQL: Remove qualifier from attrs (elastic#110581)
  Force using the last centroid during merging (elastic#111644)
  ...

# Conflicts:
#	server/src/main/java/org/elasticsearch/TransportVersions.java
#	x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceNamedWriteablesProvider.java
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this pull request Sep 4, 2024
Rename `xContent.streamSeparator()` and
`RestHandler.supportsStreamContent()` to `xContent.bulkSeparator()` and
`RestHandler.supportsBulkContent()`.

I want to reserve use of "supportsStreamContent" for current work in
HTTP layer to [support incremental content
handling](elastic#111438) besides
fully aggregated byte buffers. `supportsStreamContent` would indicate
that handler can parse chunks of http content as they arrive.
davidkyle pushed a commit to davidkyle/elasticsearch that referenced this pull request Sep 5, 2024
Rename `xContent.streamSeparator()` and
`RestHandler.supportsStreamContent()` to `xContent.bulkSeparator()` and
`RestHandler.supportsBulkContent()`.

I want to reserve use of "supportsStreamContent" for current work in
HTTP layer to [support incremental content
handling](elastic#111438) besides
fully aggregated byte buffers. `supportsStreamContent` would indicate
that handler can parse chunks of http content as they arrive.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Core/Infra/REST API REST infrastructure and utilities >refactoring Team:Core/Infra Meta label for core/infra team v8.16.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants