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

Introduce support for relative include of non-package modules ("workspaces") #273

Closed
wants to merge 41 commits into from

Conversation

DavidVujic
Copy link

@DavidVujic DavidVujic commented Jan 29, 2022

The purpose of the changes here is to enable Workspace support. A workspace is a place for code and projects. Within the workspace, code can be shared. A workspace is usually at the root of your repository.

To identify a workspace in a Python repo, an empty workspace.toml file is put at the top of the workspace. Future plugins that extends workspaces could use that file to store configuration and settings.

The feature in this pull request will make this this plugin redundant 😄 (I am the author of that plugin)

Why workspaces?
A workspace can contain more than one project. Different projects will likely use the same code. A very simplistic example would be a logger. To avoid code duplication, code could be moved out from the project into packages, and each project can reference the package from the project specific pyproject.toml file.

This requires that Poetry allows package includes (note the difference from dependencies) that are "outside" of the project path, but within a workspace. That's what this pull request will do.

An example & simplified tree workspace structure (note the namespacing for shared package includes):

projects/
  my_app/
    pyproject.toml (including a shared package)

  my_service/
    pyproject.toml (including other shared packages)

shared/
  my_namespace/
    my_package/
      __init__.py
      code.py

    my_other_package/
      __init__.py
      code.py

workspace.toml (a file that tells the plugin where to find the workspace root)

I think this feature resolves the issues raised in:
python-poetry/poetry#936
and probably also
python-poetry/poetry#2270

  • Added tests for changed code.
  • Updated documentation for changed code.

@finswimmer
Copy link
Member

Hello @DavidVujic,

thanks a lot for your contribution and feature idea. In general I like the idea of "workspaces" and think it is a valuable thing.

Currently we are working hard to get poetry 1.2 ready. Once it is ready there will be very likely a phase where we will need to stabilize all the new stuff. Once this is done we can focus on new feature again.

So please be prepared that it will take some time (unfortunately a can not say how long) until we can have a closer look on your implementation.

I'm telling you that now, because I don't want you to be frustrated, if you didn't receive feedback within a certain time from us. We really appreciated your willing to contribute to Poetry. 👍 🙏

fin swimmer

@DavidVujic
Copy link
Author

DavidVujic commented Jan 30, 2022

Hi @finswimmer,

No worries! Great job with the new stuff, I can imagine it is a lot of work. 💪

By working with this PR, I have learned more about Poetry and got some new ideas for how to develop an upcoming plugin, that aims to simplify working with Monorepos.

I will do a couple of updates on this one and additions before switching it from a draft to an official pull request.

@DavidVujic DavidVujic force-pushed the add_workspace_support branch 2 times, most recently from 2060392 to f5e36d8 Compare February 1, 2022 09:34
@DavidVujic DavidVujic marked this pull request as ready for review February 1, 2022 09:55
@DavidVujic DavidVujic changed the title WIP: Add workspace support Add workspace support Feb 1, 2022
@DavidVujic
Copy link
Author

DavidVujic commented Feb 2, 2022

I think that this need some more work. When building, the dist will contain code that are in separate folders and that will probably not work when installing the build code as a dependency (entry point, imports that are one level only).

A solution: in this commit - introducing namespaced shared package includes.

A possible solution:
The BuildIncludeFile.relative_to_source_root could return a custom path for the workspace scenario:

def relative_to_source_root(self) -> Path:
if self.workspace:
return Path(
f"{self.project_root.name}/{self.path.parent.name}/{self.path.name}"
)

That would create a dist build with "projectname/package/file.py".

Alternatively, pass in other params to BuildIncludeFile and/or call a new function from the builders.

✅ done I am working on this currently. 

@sonarcloud
Copy link

sonarcloud bot commented Feb 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@abn
Copy link
Member

abn commented Mar 25, 2022

@DavidVujic Thank you for kicking this off. This is definitely a great feature for the python ecosystem.

I have not gone into the details of implementation yet, but I amd definitely keen on getting this or a similar concept into poetry. Hence python-poetry/poetry#2270. :)

A few high level questions:

  1. Is this a feature that core needs to be aware of? If so in what scenarios? I can really only thing of a case when pip/build tries to build from sdist or repo including all code. Building a project within the workspace that depends on another under PEP517 might be a problematic notion anyway - I'd definitely like us to think about this too for whatever implementation comes about.
  2. Is a workspace.toml really required? Can we use pyproject.toml itself to not add more files developers need to deal with?
  3. How would you anticipate dependecy resolution working in workspace projects?

Asking these because I would really love to have the feature flushed out a bit more before we start the implementation.

@DavidVujic
Copy link
Author

DavidVujic commented Mar 25, 2022

@DavidVujic Thank you for kicking this off. This is definitely a great feature for the python ecosystem.

Hi!

I have released a Poetry plugin (based on the preview that has support for plugins) that takes the idea of a workspace - a monorepo - containing components and a simplistic way of reusing code in several projects. It is based on the Polylith architecture. The difference between a component and a library is that a component is much smaller, more like a LEGO brick to be used with other components 😄

In short, this approach encourages you to use the packages section in pyproject.toml where you can define dependencies, without the requirement, as of today, of being in a subfolder. Currently in the plugin, that is being done in a very hacky way (patching existing code).

I found it most simplistic to use a workspace.toml to be able to find the workspace root. When having several pyproject.toml files in a monorepo, it could be confusing when trying to figure out where the root is programatically.

A workspace.toml will also open up for third party plugins putting meta data in there. This is currently done in the Polylith Poetry plugin.

I have written a post about this and a short demo/video, explaining the idea and the tooling (based on Poetry):
https://davidvujic.blogspot.com/2022/02/a-fresh-take-on-monorepos-in-python.html

Here's the Poetry plugin repo (also published to pypi):
https://github.com/DavidVujic/poetry-polylith-plugin

@DavidVujic DavidVujic force-pushed the add_workspace_support branch 3 times, most recently from 120963b to 469878b Compare March 26, 2022 08:06
@sonarcloud
Copy link

sonarcloud bot commented Apr 3, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sonarcloud
Copy link

sonarcloud bot commented May 22, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.1% 0.1% Duplication

@DavidVujic
Copy link
Author

Hi 👋

Any chance of getting this one reviewed, and 🤞 maybe even merged? 😄

@DavidVujic DavidVujic force-pushed the add_workspace_support branch 3 times, most recently from 2c15dd6 to 8397e37 Compare August 31, 2022 15:29
@DavidVujic DavidVujic force-pushed the add_workspace_support branch 2 times, most recently from a719575 to 546743f Compare September 3, 2022 09:26
@DavidVujic DavidVujic force-pushed the add_workspace_support branch 2 times, most recently from 9a0d42b to ce925be Compare September 11, 2022 10:02
@DavidVujic
Copy link
Author

Here's a video (sound on!) where I explain the feature that is added in this Pull Request. Please let me know if you have any questions, thoughts or feedback about this.

(I had to downsize the quality to be able to post it in a comment like this)

workspaces-in-python-poetry.-.SD.480p.mov

@selvavm
Copy link

selvavm commented Sep 13, 2022

Hi @DavidVujic i am also interested in this feature and I could see you have put tremendous effort here.

I hope this is reviewed and merged.

DavidVujic and others added 26 commits October 25, 2022 13:06
…ject.toml file is. The cwd could be somewhere else if the build command would accept a path to pyproject.toml.
@sonarcloud
Copy link

sonarcloud bot commented Oct 25, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@DavidVujic
Copy link
Author

DavidVujic commented Oct 30, 2022

This Pull Request has come full circle 🟢 😄

I understand the hesitation to add the functionality of this Pull Request into poetry-core. Meanwhile, I have updated the poetry-multiproject-plugin - currently in version 1.0.4 - that adds a command called build-project. You'll find it in PyPI.

The plugin will allow relative includes in a pyproject.toml project file. What it does is:

  1. copies the project into a temporary folder.
  2. collects the relative includes - aka include = "foo/bar", from = "../../shared" - and copy into the temprary folder.
  3. generates a new pyproject.toml.
  4. runs poetry build on the temporary folder.
  5. copies the built dist folder into the original project folder.
  6. cleaning up the temporary folder.

Give it a try and please let me know if you find any 🐛 🐛 . Please add an issue to the github page of the project. 🙏

@DavidVujic DavidVujic closed this Nov 7, 2022
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.

8 participants