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

Releases/v1.26.0 branch: Implementation of SDK Version comparison for API's #60

Merged
merged 7 commits into from
Jan 3, 2024

Conversation

mrnthv
Copy link
Contributor

@mrnthv mrnthv commented Nov 16, 2023

Jira ticket: https://sightmachine.atlassian.net/browse/ENG-3221

  1. Implementation based on https://sightmachine.atlassian.net/l/cp/gX7TxZUU (SDK Release Process)
  2. Added release version check decorator to all SDK API functions
  3. Added a new _version.py file to track the version. This version will be compared against the latest version of github release of sightmachine-sdk. Warning will be printed only when the installed version is not the latest.
  4. Added a CHANGELOG.md file to capture the release features and bug fixes.
  5. Added requirements.txt to MANIFEST.in to enable installation from package.

@codecov-commenter
Copy link

codecov-commenter commented Nov 16, 2023

Codecov Report

Attention: 28 lines in your changes are missing coverage. Please review.

Comparison is base (79d8e5a) 20.30% compared to head (96e3932) 23.53%.

Files Patch % Lines
smsdk/_version.py 54.83% 28 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
+ Coverage   20.30%   23.53%   +3.23%     
==========================================
  Files          10       11       +1     
  Lines        1261     1351      +90     
  Branches      279      315      +36     
==========================================
+ Hits          256      318      +62     
- Misses       1005     1033      +28     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mrnthv mrnthv changed the title Releases/v1.26.0 Releases/v1.26.0 branch: Implementation of SDK Version comparison for API's Nov 22, 2023
Copy link
Member

@demaagdk demaagdk left a comment

Choose a reason for hiding this comment

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

This looks fine to me

smsdk/_version.py Show resolved Hide resolved
smsdk/_version.py Outdated Show resolved Hide resolved
Copy link

@mklein0 mklein0 left a comment

Choose a reason for hiding this comment

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

Minor comments on typing and some suggested optimizations. Looks good.

setup.py Outdated
Copy link

Choose a reason for hiding this comment

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

FYI, I am going to suggest the creation of a new ticket to convert the python packaging from setup.py to the new pyproject.toml build system. The setup.py system has been deprecated.

smsdk/_version.py Outdated Show resolved Hide resolved
smsdk/_version.py Outdated Show resolved Hide resolved
smsdk/_version.py Outdated Show resolved Hide resolved
smsdk/client.py Outdated Show resolved Hide resolved
smsdk/client.py Outdated Show resolved Hide resolved
smsdk/client.py Outdated Show resolved Hide resolved
smsdk/_version.py Outdated Show resolved Hide resolved
smsdk/_version.py Show resolved Hide resolved
Copy link

@mklein0 mklein0 left a comment

Choose a reason for hiding this comment

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

Looks good.

Comment on lines +78 to +79
api_version_printed = False
last_version_check_time = None
Copy link

Choose a reason for hiding this comment

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

FYI, do these need to be separate flags? Could have a method/property on the class which does the equivalent.

@JMkrish JMkrish merged commit 219b718 into master Jan 3, 2024
6 checks passed
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.

5 participants