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.

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

Populate WorkerMetadata in WorkerInitResponse message #821

Closed
liliankasem opened this issue Jun 9, 2022 · 10 comments · Fixed by #884
Closed

Populate WorkerMetadata in WorkerInitResponse message #821

liliankasem opened this issue Jun 9, 2022 · 10 comments · Fixed by #884
Assignees
Labels

Comments

@liliankasem
Copy link
Member

liliankasem commented Jun 9, 2022

The protobuf has been updated to include a WorkerMetadata property within the WorkerInitResponse message. The worker should include WorkerMetadata when sending the WorkerInitResponse to the host.

PR for reference on how to fill in the fields.

You will need to pull in the latest protobuf version first (this doc may help).

message WorkerMetadata {
  // The runtime/stack name
  string runtime_name = 1;

  // The version of the runtime/stack
  string runtime_version = 2;

  // The version of the worker or worker SDK
  string worker_version = 3;

  // The worker bitness/architecture
  string worker_bitness = 4;

  // Optional additional custom properties
  map<string, string> custom_properties = 5;
}
@Francisco-Gamino Francisco-Gamino self-assigned this Jun 10, 2022
@liliankasem liliankasem changed the title Populate worker_version in WorkerInitResponse message Populate WorkerMetadata in WorkerInitResponse message Jun 14, 2022
@liliankasem
Copy link
Member Author

liliankasem commented Jun 15, 2022

Latest proto version is now v1.5.8

Edit: I had to force push so please confirm the commit hash is 14b2ba5 when you pull the protobuf into your repo

@liliankasem
Copy link
Member Author

@Francisco-Gamino can we prioritize this for the next sprint?

@liliankasem
Copy link
Member Author

@Francisco-Gamino
Copy link
Contributor

@Francisco-Gamino can we prioritize this for the next sprint?

@liliankasem -- Yes, we will include this fix in the next sprint.

@liliankasem
Copy link
Member Author

@Francisco-Gamino can we prioritize this for the next sprint?

@liliankasem -- Yes, we will include this fix in the next sprint.

Thank you!

@Francisco-Gamino Francisco-Gamino added P1 and removed P2 labels Oct 15, 2022
@Francisco-Gamino
Copy link
Contributor

Adding prerequisite for this work item: #875.

@Francisco-Gamino
Copy link
Contributor

Adding prerequisite for this work item: #875.

prerequisite work item has been completed.

@Francisco-Gamino
Copy link
Contributor

@liliankasem - In order to populate the language worker RuntimeVersion, we need to initialize the worker to use the PowerShell SDK (either 7.2 or 7.0). Currently, this initialization takes place in the first ProcessFunctionLoadRequest once we know the functionAppRoot and whether the customer has enabled Managed Dependencies. However, to complete this work item, we need to move the initialization of the worker to ProcessWorkerInitRequest. After this change, we need to test in prod to make sure no regression are introduced. I will discuss with this with the team in our next sync up and provide a new ETA.

/cc @AnatoliB @michaelpeng36

@Francisco-Gamino
Copy link
Contributor

@liliankasem -- The next deployment (which starts January 3rd) of the PowerShell worker 7.2 will have this fix.

@liliankasem
Copy link
Member Author

@liliankasem -- The next deployment (which starts January 3rd) of the PowerShell worker 7.2 will have this fix.

Great! thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants