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

[Feature] Deprecation messages for invalid imports #94

Merged
merged 9 commits into from
Feb 10, 2024

Conversation

MHHukiewitz
Copy link
Member

Update Aleph SDK client to raise ImportError with appropriate error message for deprecated classes and types.

Copy link

The PR introduces a significant amount of changes to the aleph-sdk-python library, including deprecation and removal of several classes and methods. This is likely to have a high impact on the codebase as it involves major refactoring and potential breaking changes.

Here are some key points:

  1. The AlephClient and AuthenticatedAlephClient classes have been deprecated, with suggested replacements being AlephHttpClient and AuthenticatedAlephHttpClient respectively. This could potentially break existing code that directly imports these classes.
  2. The synchronous and asynchronous types have also been removed from the library.
  3. Several methods in the AbstractAlephClient class are now raising an ImportError suggesting they should be imported as AlephHttpClient or AuthenticatedAlephHttpClient instead.
  4. The PR introduces a new method ipfs_push(self, content: Mapping) -> str which is likely to have implications for the IPFS integration in the library.
  5. There are also several tests added and existing ones modified, suggesting that thorough testing has been considered before merging this PR.

Given these points, it's recommended to review this PR carefully and consider its potential impact on other parts of the codebase or future development. The PR author should be consulted for any specific rules or guidelines related to this categorization.

Please note that while I have provided a general assessment based on the diff and the changes, it's always best to discuss these changes with the team responsible for maintaining the library to ensure they are compatible with your project's needs.

@github-actions github-actions bot added the BLACK This PR has critical implications and must be reviewed by a senior engineer. label Jan 30, 2024
src/aleph/sdk/__init__.py Outdated Show resolved Hide resolved
nesitor
nesitor previously approved these changes Feb 1, 2024
MHHukiewitz and others added 3 commits February 1, 2024 13:25
Solution: loosen it to accept compatible versions to 0.4.2
Co-authored-by: nesitor <[email protected]>
Co-authored-by: Hugo Herter <[email protected]>
@hoh
Copy link
Member

hoh commented Feb 9, 2024

Needs squashing ;-)

@MHHukiewitz MHHukiewitz merged commit 31c816b into main Feb 10, 2024
11 checks passed
@MHHukiewitz
Copy link
Member Author

A shit, I merged it with the GitHub Mobile App, and it just went the lazy route T.T

@MHHukiewitz MHHukiewitz deleted the mhh-add-deprecation-message branch February 12, 2024 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BLACK This PR has critical implications and must be reviewed by a senior engineer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants