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

[SYNPY-1344] Adding activity model for OOP #1055

Merged
merged 11 commits into from
Jan 29, 2024

Conversation

BryanFauble
Copy link
Contributor

Problem:

  1. We need to create an OOP approach to working with the Activity model.
  2. Some documentation needed to be updated
  3. Remove OTEL from the test scripts showing how the interfaces are working

Solution:

  1. Creating the Activity model based off the Rest API
  2. Updating documentation
  3. Creating a test script

Testing:

  1. Unit and integration tests
  2. The oop_poc_activity.py script

@BryanFauble BryanFauble requested a review from a team as a code owner January 25, 2024 20:23
@pep8speaks
Copy link

pep8speaks commented Jan 25, 2024

Hello @BryanFauble! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 2722:89: E501 line too long (111 > 88 characters)
Line 2723:89: E501 line too long (106 > 88 characters)
Line 2756:89: E501 line too long (111 > 88 characters)
Line 2757:89: E501 line too long (106 > 88 characters)
Line 2784:89: E501 line too long (111 > 88 characters)
Line 2785:89: E501 line too long (106 > 88 characters)
Line 2796:89: E501 line too long (90 > 88 characters)
Line 2797:89: E501 line too long (110 > 88 characters)
Line 2826:89: E501 line too long (111 > 88 characters)
Line 2827:89: E501 line too long (106 > 88 characters)

Line 1232:89: E501 line too long (89 > 88 characters)
Line 1233:89: E501 line too long (90 > 88 characters)
Line 1240:89: E501 line too long (97 > 88 characters)
Line 1244:89: E501 line too long (92 > 88 characters)

Line 31:89: E501 line too long (95 > 88 characters)
Line 89:89: E501 line too long (90 > 88 characters)
Line 90:89: E501 line too long (97 > 88 characters)
Line 111:89: E501 line too long (92 > 88 characters)
Line 183:89: E501 line too long (92 > 88 characters)
Line 241:89: E501 line too long (110 > 88 characters)
Line 306:89: E501 line too long (110 > 88 characters)
Line 344:89: E501 line too long (110 > 88 characters)

Line 214:14: F841 local variable 'ex' is assigned to but never used

Line 25:89: E501 line too long (91 > 88 characters)
Line 41:89: E501 line too long (91 > 88 characters)
Line 63:89: E501 line too long (91 > 88 characters)
Line 79:89: E501 line too long (91 > 88 characters)

Comment last updated at 2024-01-26 17:06:39 UTC

Copy link
Collaborator

@jaymedina jaymedina 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 at first glance, I made some small comments, most are just for clarity, feel free to ignore some of them.

Copy link
Member

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

🔥 Wonderful PR. I left some comments, but I'm going to pre-approve. Most of them are Nits and/or potentially different Jira tickets.

synapseclient/models/activity.py Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

I'll leave it to the team to validate these in the feature branch. I will be running through these after they are merged into develop / client release, but awesome work here!

synapseclient/core/utils.py Outdated Show resolved Hide resolved
synapseclient/models/activity.py Show resolved Hide resolved
synapseclient/models/activity.py Show resolved Hide resolved
synapseclient/models/activity.py Outdated Show resolved Hide resolved
synapseclient/models/activity.py Outdated Show resolved Hide resolved
synapseclient/models/file.py Show resolved Hide resolved
Copy link

sonarcloud bot commented Jan 26, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

12 New issues
0 Security Hotspots
82.6% Coverage on New Code
15.1% Duplication on New Code

See analysis details on SonarCloud

@BryanFauble BryanFauble merged commit ddc935b into develop Jan 29, 2024
38 checks passed
@BryanFauble BryanFauble deleted the SYNPY-1344-activity-model branch January 29, 2024 18:14
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.

None yet

5 participants