Skip to content

Adding temporalio - #30569

Merged
danielnachun merged 9 commits into
conda-forge:mainfrom
chrisjryan:temporalio
Aug 22, 2025
Merged

Adding temporalio#30569
danielnachun merged 9 commits into
conda-forge:mainfrom
chrisjryan:temporalio

Conversation

@chrisjryan

@chrisjryan chrisjryan commented Jul 18, 2025

Copy link
Copy Markdown
Contributor

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@github-actions

github-actions Bot commented Jul 18, 2025

Copy link
Copy Markdown
Contributor

Hi! This is the staged-recipes linter and I found some lint.

File-specific lints and/or hints:

  • recipes/temporalio/meta.yaml:
    • lints:
      • The following maintainers have not yet confirmed that they are willing to be listed here: fzding, dgasmith, nic-chira. Please ask them to comment on this PR if they are.

@conda-forge-admin

conda-forge-admin commented Jul 18, 2025

Copy link
Copy Markdown
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/temporalio/meta.yaml) and found some lint.

Here's what I've got...

For recipes/temporalio/meta.yaml:

  • ❌ The home item is expected in the about section.
  • ❌ Non noarch packages should have python requirement without any version constraints.
  • ❌ Non noarch packages should have python requirement without any version constraints.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/16380234400. Examine the logs at this URL for more detail.

The output related to the `maturin` and Rust compilation, as well as protobufs.
@chrisjryan chrisjryan changed the title Add temporalio Adding temporalio Jul 18, 2025
@fzding

fzding commented Jul 18, 2025

Copy link
Copy Markdown

I am willing to be a maintainer.

@nic-chira

Copy link
Copy Markdown

willing to help maintain

@dgasmith

Copy link
Copy Markdown
Contributor

I can help maintain!

@github-actions

Copy link
Copy Markdown
Contributor

Hi! This is the staged-recipes linter and your PR looks excellent! 🚀

@conda-forge-admin

Copy link
Copy Markdown
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/temporalio/meta.yaml) and found it was in an excellent condition.

@chrisjryan
chrisjryan marked this pull request as ready for review July 21, 2025 21:39
@chrisjryan

Copy link
Copy Markdown
Contributor Author

@conda-forge/help-python ready to review!

Can someone advise on how I can verify if this PR passes the two "static libraries" items on the checklist? I'm new to contributing to conda-forge and am not 100% clear on how to make sure of this. Thanks!

@danielnachun danielnachun left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've suggested some changes to get the third party license information added to the recipe output.

Comment thread recipes/temporalio/meta.yaml Outdated
Comment thread recipes/temporalio/meta.yaml
Comment thread recipes/temporalio/meta.yaml Outdated
@danielnachun

Copy link
Copy Markdown
Contributor

Can someone advise on how I can verify if this PR passes the two "static libraries" items on the checklist? I'm new to contributing to conda-forge and am not 100% clear on how to make sure of this. Thanks!

This concern doesn't apply here because the native code is producing a shared library as seen in the build logs - it ends in .so for Linux, .dylib for macOS and .dll for Windows.

@danielnachun

Copy link
Copy Markdown
Contributor

@xhochy I'm not sure this is a blocker, but do we have a way to get auditable binaries for a Python package with embedded Rust like this? It is becoming increasingly common for Python, NodeJS and even C/C++ projects to embed Rust code and it's not terribly clear to me yet how we can make sure the generated binaries are auditable.

@xhochy

xhochy commented Jul 23, 2025

Copy link
Copy Markdown
Member

At least for maturin, I verified that this works: conda-forge/cfep#63

My aim is to push that CFEP in September.

Co-authored-by: Daniel Nachun <daniel.nachun@gmail.com>
Comment thread recipes/temporalio/meta.yaml Outdated
@danielnachun

Copy link
Copy Markdown
Contributor

At least for maturin, I verified that this works: conda-forge/cfep#63

My aim is to push that CFEP in September.

That's good to know. I guess ideally build systems will respect $CARGO, and if they don't, it may be possible to create a wrapper script around cargo auditable and prepend that to PATH if the build is hard-coded to use the cargo executable. I will follow up in the relevant issues so we're aware that we need to be thinking about this for recipe reviews.

@chrisjryan

chrisjryan commented Jul 25, 2025

Copy link
Copy Markdown
Contributor Author

Thanks for the review @danielnachun , I incorporated and adapted your suggested change & saw this pass locally with linux64. However, it seems to have failed in the linux64 Azure pipeline with a 502 error when downloading one of the dependencies – maybe this is intermittent & it's worth retrying the pipeline? It also failed during the win64 build when the worker ran out of disk space, but I'm not sure what to do about that. What would you advise?

@chrisjryan
chrisjryan requested a review from danielnachun July 25, 2025 17:41
@dgasmith

dgasmith commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

@conda-forge/help-rust Pinging a slightly larger crowd here, I believe @chrisjryan's comment is accurate based off a review of the logs.

@github-actions github-actions Bot added the R label Jul 30, 2025
@chrisjryan

Copy link
Copy Markdown
Contributor Author

Hi @danielnachun and @conda-forge/help-rust , just checking in again. Do you have any advice around our questions in the comments above?

@danielnachun

Copy link
Copy Markdown
Contributor

Make a new commit to skip Windows - that should also rerun the CI and hopefully get past the transient issue that failed last time on Linux.

Comment thread recipes/temporalio/meta.yaml Outdated
@chrisjryan

Copy link
Copy Markdown
Contributor Author

Hi again @danielnachun , just checking back in in case you have time to review. Your suggested change from last week has allowed test to pass.

@danielnachun danielnachun left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just one minor tweak and then we can merge.

Comment thread recipes/temporalio/meta.yaml Outdated
since Python 3.8 is no longer supported anyway

Co-authored-by: Daniel Nachun <daniel.nachun@gmail.com>
@chrisjryan

Copy link
Copy Markdown
Contributor Author

@danielnachun Tweak added! Thanks again for the help.

@danielnachun
danielnachun merged commit 226216b into conda-forge:main Aug 22, 2025
7 checks passed
@chrisjryan
chrisjryan deleted the temporalio branch August 25, 2025 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants