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

Clean up and correct properties to producer and consumers created by Functions/Sinks/Sources #3315

Merged
merged 4 commits into from Jan 7, 2019
Merged

Clean up and correct properties to producer and consumers created by Functions/Sinks/Sources #3315

merged 4 commits into from Jan 7, 2019

Conversation

jerrypeng
Copy link
Contributor

@jerrypeng jerrypeng commented Jan 7, 2019 •
edited

Motivation

The metadata specified in producers and consumers created by functions, sinks, and sources can help with debugging.

This PR aims improving that aspect as well as add the metadata to python functions as well

Modifications

Example of what the meta looks like:

        "metadata" : {
          "instance_id" : "0",
          "application" : "pulsar-function",
          "id" : "public/default/test2"
        }

@jerrypeng jerrypeng changed the title Functions consumer producer props Clean up and correct properties to producer and consumers created by Functions/Sinks/Sources Jan 7, 2019
@jerrypeng
Copy link
Contributor Author

@cckellogg please take a look as well

cckellogg
cckellogg reviewed Jan 7, 2019
View reviewed changes
pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/InstanceUtils.java Outdated
properties.put("application", "pulsar-sink");
break;
}
properties.put("id", fullyQualifiedInstanceId);
Copy link
Contributor

Choose a reason for hiding this comment

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

should we have a name field too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could have separate fields for tenant, namespace, and name. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think just a field with fully qualified name is fine
maybe:
application = pulsar-function|pulsar-sink|pulsar-source
id = tenant/namespace/name
instanceId = instanceId

what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cckellogg sounds good. I have updated the PR

@jerrypeng jerrypeng self-assigned this Jan 7, 2019
@jerrypeng jerrypeng added this to the 2.3.0 milestone Jan 7, 2019
@jerrypeng jerrypeng merged commit 7b239ec into apache:master Jan 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cckellogg cckellogg cckellogg left review comments

@merlimat merlimat merlimat approved these changes

@sijie sijie Awaiting requested review from sijie

@srkukarni srkukarni Awaiting requested review from srkukarni

Assignees

@jerrypeng jerrypeng

Labels
area/function
Projects
None yet
Milestone
2.3.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jerrypeng @merlimat @cckellogg