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

Packaging this sdk with PEX causes collisions #281

Closed
Sheemap opened this issue Apr 4, 2024 · 3 comments
Closed

Packaging this sdk with PEX causes collisions #281

Sheemap opened this issue Apr 4, 2024 · 3 comments

Comments

@Sheemap
Copy link

Sheemap commented Apr 4, 2024

Is your feature request related to a problem? Please describe.

Yes, It's causing an issue with our build system. The issue is the testing directories for this repo and the python-eventsource repo, are named the same and both included in their respective pip releases.

We use the pex build tool to package up all our dependencies up into a serverless function, it tries to ensure packages don't conflict when it collects those dependencies.

Unfortunately when building python-server-sdk, it errors out due to a namespace collision with python-eventsource, which it depends on.

I got to this conclusion from someone else reporting this error to the Pants build system devs, here is the maintainer's explanation and proposed fix. pantsbuild/pants#20224 (comment)

Describe the solution you'd like

If it isn't used at runtime, excluding the testing folder from the released pip package would be great.

Describe alternatives you've considered

  • Move the test folders to be within the ldclient module
  • Renaming the folders

Additional context

There is a workaround we can use short term to allow collisions, but we hope to use launchdarkly on a lot of functions, and don't want to disable this collision feature long term.

Logs I'm seeing

Encountered collisions populating src.python.api/lambda from PEX at faas_repository.pex:
1. src.python.api/lambda/testing/http_util.py was provided by:
	sha1:41a85a0d0ed3f173204af13855221a85262e9509 -> /home/bread/.cache/pants/named_caches/pex_root/installed_wheels/1b9580d49755735bd7acccd4e2cecfc17a1669e8e499e96e396a401012dbdae3/launchdarkly_server_sdk-9.3.0-py3-none-any.whl/testing/http_util.py
	sha1:6992afbf74d310b1d70287f856d9d96140f9455d -> /home/bread/.cache/pants/named_caches/pex_root/installed_wheels/3d7e5301bc4b4a744ecdaa10de8bce52c2f3c66a97e9aa10ab11ca81b67fb31b/launchdarkly_eventsource-1.1.1-py3-none-any.whl/testing/http_util.py
2. src.python.api/lambda/testing/__init__.py was provided by:
	sha1:b679ff39746fd1ccd574ad90603eb8c5e97d5164 -> /home/bread/.cache/pants/named_caches/pex_root/installed_wheels/1b9580d49755735bd7acccd4e2cecfc17a1669e8e499e96e396a401012dbdae3/launchdarkly_server_sdk-9.3.0-py3-none-any.whl/testing/__init__.py
	sha1:da39a3ee5e6b4b0d3255bfef95601890afd80709 -> /home/bread/.cache/pants/named_caches/pex_root/installed_wheels/3d7e5301bc4b4a744ecdaa10de8bce52c2f3c66a97e9aa10ab11ca81b67fb31b/launchdarkly_eventsource-1.1.1-py3-none-any.whl/testing/__init__.py

Thanks!
@Sheemap Sheemap changed the title Building this package with pex causes collisions Packaging this sdk with PEX causes collisions Apr 4, 2024
@keelerm84
Copy link
Member

Thank you for bringing this to our attention. I will take a look at this and let you know once we have a resolution.

Tracking internally as sc-239226.

keelerm84 added a commit to launchdarkly/python-eventsource that referenced this issue Apr 4, 2024
@keelerm84
fix: Move tests under ld_eventsource namespace
f659746 
Previous distributions of this package included two packages --
ld_eventsource and testing. This top level testing namespace can
conflict with other packages. In fact, it conflicts with our own SDK.

In general this doesn't matter, but it may if:

1. You are using a build process that warns about conflicts (see [this
   issue][1])
2. You want to install the sdist on an unsupported platform and would
   like to be able to verify the tests.

To resolve this issue, we are moving the testing folder into the
ld_eventsource package. These testing files will only be included in the
sdist format. This allows for a smaller wheel size while also allowing
for flexibility with consumers.

[1]: launchdarkly/python-server-sdk#281
@keelerm84 keelerm84 mentioned this issue Apr 4, 2024
Merged
keelerm84 added a commit to launchdarkly/python-eventsource that referenced this issue Apr 4, 2024
@keelerm84
fix: Move tests under ld_eventsource namespace
6e2d363 
Previous distributions of this package included two packages --
ld_eventsource and testing. This top level testing namespace can
conflict with other packages. In fact, it conflicts with our own SDK.

In general this doesn't matter, but it may if:

1. You are using a build process that warns about conflicts (see [this
   issue][1])
2. You want to install the sdist on an unsupported platform and would
   like to be able to verify the tests.

To resolve this issue, we are moving the testing folder into the
ld_eventsource package. These testing files will only be included in the
sdist format. This allows for a smaller wheel size while also allowing
for flexibility with consumers.

[1]: launchdarkly/python-server-sdk#281
keelerm84 added a commit to launchdarkly/python-eventsource that referenced this issue Apr 4, 2024
@keelerm84
fix: Move tests under ld_eventsource namespace
7bb69a0 
Previous distributions of this package included two packages --
ld_eventsource and testing. This top level testing namespace can
conflict with other packages. In fact, it conflicts with our own SDK.

In general this doesn't matter, but it may if:

1. You are using a build process that warns about conflicts (see [this
   issue][1])
2. You want to install the sdist on an unsupported platform and would
   like to be able to verify the tests.

To resolve this issue, we are moving the testing folder into the
ld_eventsource package. These testing files will only be included in the
sdist format. This allows for a smaller wheel size while also allowing
for flexibility with consumers.

[1]: launchdarkly/python-server-sdk#281
keelerm84 added a commit to launchdarkly/python-eventsource that referenced this issue Apr 4, 2024
@keelerm84
fix: Move tests under ld_eventsource namespace
d7d12c8 
Previous distributions of this package included two packages --
ld_eventsource and testing. This top level testing namespace can
conflict with other packages. In fact, it conflicts with our own SDK.

In general this doesn't matter, but it may if:

1. You are using a build process that warns about conflicts (see [this
   issue][1])
2. You want to install the sdist on an unsupported platform and would
   like to be able to verify the tests.

To resolve this issue, we are moving the testing folder into the
ld_eventsource package. These testing files will only be included in the
sdist format. This allows for a smaller wheel size while also allowing
for flexibility with consumers.

[1]: launchdarkly/python-server-sdk#281
keelerm84 added a commit to launchdarkly/python-eventsource that referenced this issue Apr 4, 2024
@keelerm84
fix: Move tests under ld_eventsource namespace (#29)
74a41db 
Previous distributions of this package included two packages --
ld_eventsource and testing. This top level testing namespace can
conflict with other packages. In fact, it conflicts with our own SDK.

In general this doesn't matter, but it may if:

1. You are using a build process that warns about conflicts (see [this
   issue][1])
2. You want to install the sdist on an unsupported platform and would
   like to be able to verify the tests.

To resolve this issue, we are moving the testing folder into the
ld_eventsource package. These testing files will only be included in the
sdist format. This allows for a smaller wheel size while also allowing
for flexibility with consumers.

[1]: launchdarkly/python-server-sdk#281
keelerm84 added a commit that referenced this issue Apr 4, 2024
@keelerm84
fix: Move tests under ldclient namespace (#29)
5dccb43 
Previous distributions of this package included two packages -- ldclient
and testing. This top level testing namespace can conflict with other
packages. In fact, it conflicts with our own eventsource library.

In general this doesn't matter, but it may if:

1. You are using a build process that warns about conflicts (see [this
   issue][1])
2. You want to install the sdist on an unsupported platform and would
   like to be able to verify the tests.

To resolve this issue, we are moving the testing folder into the
ldclient package. These testing files will only be included in the sdist
format. This allows for a smaller wheel size while also allowing for
flexibility with consumers.

[1]: #281
keelerm84 added a commit that referenced this issue Apr 4, 2024
@keelerm84
fix: Move tests under ldclient namespace (#29)
6a71a0a 
Previous distributions of this package included two packages -- ldclient
and testing. This top level testing namespace can conflict with other
packages. In fact, it conflicts with our own eventsource library.

In general this doesn't matter, but it may if:

1. You are using a build process that warns about conflicts (see [this
   issue][1])
2. You want to install the sdist on an unsupported platform and would
   like to be able to verify the tests.

To resolve this issue, we are moving the testing folder into the
ldclient package. These testing files will only be included in the sdist
format. This allows for a smaller wheel size while also allowing for
flexibility with consumers.

[1]: #281
keelerm84 added a commit that referenced this issue Apr 4, 2024
@keelerm84
fix: Move tests under ldclient namespace (#29) (#283)
c7d367a 
Previous distributions of this package included two packages -- ldclient
and testing. This top level testing namespace can conflict with other
packages. In fact, it conflicts with our own eventsource library.

In general this doesn't matter, but it may if:

1. You are using a build process that warns about conflicts (see [this
issue][1])
2. You want to install the sdist on an unsupported platform and would
like to be able to verify the tests.

To resolve this issue, we are moving the testing folder into the
ldclient package. These testing files will only be included in the sdist
format. This allows for a smaller wheel size while also allowing for
flexibility with consumers.

[1]: #281
@keelerm84
Copy link
Member

@Sheemap this should be resolved in v9.3.1. Please let me know if you experience any further issues.

@Sheemap
Copy link
Author

Sheemap commented Apr 5, 2024

Awesome! Just confirmed it working, thanks a bunch for the quick response and fix! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Sheemap @keelerm84