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

Add baseUri path also when creating _bulk uri #3020

Closed
wants to merge 1 commit into from
Closed

Add baseUri path also when creating _bulk uri #3020

wants to merge 1 commit into from

Conversation

pramodShehan5
Copy link

@pramodShehan5 pramodShehan5 commented Oct 5, 2023 •
edited by ennru
Loading

@lightbend-cla-validator
Copy link

Hi @pramodShehan5,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

https://www.lightbend.com/contribute/cla

@pramodShehan5
Copy link
Author

This is the exact issue.
#3019

@ennru
Copy link
Member

ennru commented Nov 22, 2023

CLA was signed.

ennru
ennru reviewed Nov 22, 2023
View reviewed changes
...rch/src/main/scala/akka/stream/alpakka/elasticsearch/impl/ElasticsearchSimpleFlowStage.scala
@@ -79,7 +79,7 @@ private[elasticsearch] final class ElasticsearchSimpleFlowStage[T, C](
log.debug("Posting data to Elasticsearch: {}", json)

if (json.nonEmpty) {
val uri = baseUri.withPath(Path(endpoint))
val uri = baseUri.withPath(Path(baseUri.path.toString() + endpoint))
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look right, baseUri should already be there as this attaches to it.

Copy link
Author

Choose a reason for hiding this comment

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

if baseUri is https://test.com:9200/elasticsearch, it is ignoring /elasticsearch path

Copy link
Author

Choose a reason for hiding this comment

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

image

Copy link
Member

Choose a reason for hiding this comment

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

I believe what you are looking for is baseUri.withPath(baseUri.path / endpoint) but can't verify that right now.

@pramodShehan5 pramodShehan5 requested a review from ennru November 22, 2023 13:23
@ennru
Copy link
Member

ennru commented Dec 1, 2023

Superseded by

@ennru ennru closed this Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ennru ennru Awaiting requested review from ennru

Assignees
No one assigned
Labels
p:elasticsearch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pramodShehan5 @lightbend-cla-validator @ennru