Skip to content

Conversation

@isc-jlechtne
Copy link
Collaborator

@isc-jlechtne isc-jlechtne commented Jan 5, 2026

Addresses #1024 as well as adding the -export-python-deps flag to the "publish" command

Copy link
Collaborator

@isc-jili isc-jili left a comment

Choose a reason for hiding this comment

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

Changes look good! Thanks for fixing this.
Would it be good to add some tests for the publish command with "export-python-deps"? As well as add/edit test cases that demonstrate the previously buggy behavior that were not caught by previous existing tests?

@isc-dchui
Copy link
Collaborator

Agreed that some tests would be good. Also maybe rename the PR to something more informative?

@isc-jlechtne isc-jlechtne changed the title Initial commit for checks checks to run Publishing a module with python dependencies Jan 7, 2026
isc-eneil
isc-eneil previously approved these changes Jan 9, 2026
do ..ExportSingleModule(.tSingleModuleArray, pTargetDirectory, .pDependencyGraph, .tParams, tVerbose)
}

set exportPythonDependencies = $get(pParams("ExportPythonDependencies"), 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@isc-jlechtne Does this ensure python dependencies of dependent modules are also exported? It doesn't seem to and only seems to do it for the base module?

do $$$AssertStatusOK(sc, "Removed local filesystem repo")
set sc = ##class(%IPM.Main).Shell("repo -delete -n remote")
do $$$AssertStatusOK(sc, "Removed remote repo")
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion to add a test case asserting that wheels are:

  1. bundled into the published artifact, and
  2. absent when the flag is omitted

@isc-jlechtne isc-jlechtne marked this pull request as draft January 13, 2026 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants