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

[BUG] Undocumented breaking change in v3.21.0 - unable to import NOT_SET from schemathesis.utils #1890

Closed
3 tasks done
allanlewis opened this issue Nov 15, 2023 · 6 comments
Closed
3 tasks done
Assignees
Labels
Status: Needs Design Issue requires more design work Type: Bug Errors or unexpected behavior Type: Compatibility Compatibility with other tools Type: Documentation Documentation improvements

Comments

@allanlewis
Copy link
Contributor

Checklist

  • I checked the FAQ section of the documentation
  • I looked for similar issues in the issue tracker
  • I am using the latest version of Schemathesis

Describe the bug

My project compares Case.body to NOT_SET to determine if the case has a body. Commit dfbcce1 moved schemathesis.utils.NOT_SET to schemathesis.constants. As my use of NOT_SET is in a library and components using that library use different minor versions of schemathesis, I'm unable to upgrade to 3.21 without upgrading all of them at the same time, which I'd like to avoid.

In general, this appears to be an undocumented breaking change, the only mention being this line in the changelog:

INTERNAL: Moved heavy imports inside functions to improve CLI startup time by 4.3x, not affecting overall execution speed. #1509

To Reproduce

🚨 Mandatory 🚨: Steps to reproduce the behavior:

  1. Install schemathesis==3.20.*.
  2. Import NOT_SET from schemathesis.utils.
  3. Upgrade to schemathesis==3.21.0.
  4. See that the same code raises an ImportError due to dfbcce1.

Please include a minimal API schema causing this issue:

N/A

Expected behavior

No breaking changes between minor versions; I should be able to import NOT_SET from schemathesis.utils.

Environment

N/A

Additional context

If NOT_SET is not part of the public API, perhaps it should be moved into a private (i.e. underscore-prefixed) module, or prefixed with an underscore itself. (If we're using SemVer or similar here, that change should wait for v4.)

@allanlewis allanlewis added Status: Needs Triage Requires initial assessment to categorize and prioritize Type: Bug Errors or unexpected behavior labels Nov 15, 2023
@Stranger6667
Copy link
Member

Hi @allanlewis, NOT_SET is indeed not a part of the public API, i.e. it is not listed in the Public API reference here. The approach to handling the internal/public details was not properly documented, however, my view was - that everything beyond the documented public API is private.

That is definitely something that should be addressed clearly not only with documentation but also with a better module structure & naming. As a temporary measure, I'll restore the ability to import NOT_SET from utils and then further public/private API discussion may be postponed to v4.

@Stranger6667 Stranger6667 added Type: Documentation Documentation improvements Type: Compatibility Compatibility with other tools Status: Needs Design Issue requires more design work and removed Status: Needs Triage Requires initial assessment to categorize and prioritize labels Nov 15, 2023
@allanlewis
Copy link
Contributor Author

...NOT_SET is indeed not a part of the public API, i.e. it is not listed in the Public API reference here.

Yes, I'd noticed that. Perhaps NOT_SET should be added to the public API since it seems like a useful thing to be able to import.

As a temporary measure, I'll restore the ability to import NOT_SET from utils...

Thanks, that would be great. Shall I expect that to be included in 3.21.1?

@Stranger6667
Copy link
Member

Yes, I'd noticed that. Perhaps NOT_SET should be added to the public API since it seems like a useful thing to be able to import.

Agree! Could you share some details about your use case? maybe some other things could be exposed as well, or at least, structured better.

Thanks, that would be great. Shall I expect that to be included in 3.21.1?

Yes! :)

@Stranger6667
Copy link
Member

Btw, 3.21.1 is released

@allanlewis
Copy link
Contributor Author

Should this be resolved?

@Stranger6667
Copy link
Member

Yep!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Design Issue requires more design work Type: Bug Errors or unexpected behavior Type: Compatibility Compatibility with other tools Type: Documentation Documentation improvements
Projects
None yet
Development

No branches or pull requests

2 participants