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

Devel #1302

Conversation

JoshuaSBrown
Copy link
Collaborator

@JoshuaSBrown JoshuaSBrown commented Feb 11, 2025

PR Description

Update Tony branch from devel.

Tasks

  • - A description of the PR has been provided, and a diagram included if it is a new feature.
  • - Formatter has been run
  • - CHANGELOG comment has been added
  • - Labels have been assigned to the pr
  • - A reviwer has been added
  • - A user has been assigned to work on the pr
  • - If new feature a unit test has been added

Summary by Sourcery

Refactor code, fix bugs and update dependencies.

Bug Fixes:

  • Fix bug in authz library parsing of authz.conf file where Globus paths were incorrectly sanitized when using just '/'.
  • Fix bug in libauthz where log_path variable was not being read from the config file.
  • Fix unbound variable in Foxx entrypoint file.
  • Fix Jupyter notebooks in documentation.

Enhancements:

  • Refactor authz Foxx module, split into objects and added unit tests.
  • Fix deprecated method usage for Protobuf in Python client library.

Tests:

  • Add authz unit testing to the CI.

nedvedba and others added 29 commits January 16, 2025 05:47
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
…hema

Update the schema documentation to improve the usability.
Add github action for linting jupyter
Applied fix to deprecation warning
Fix bug in CI scripts associated with repo and gcs image build
Copy link

sourcery-ai bot commented Feb 11, 2025

Reviewer's Guide by Sourcery

This pull request implements multiple improvements throughout the repository. Major updates include significant modifications to nearly all the Jupyter notebooks (updating content, exercise instructions, formatting and documentation), enhancements to CI/CD and build configurations, library and dependency upgrades (e.g. updating protobuf from 4.21.1 to 5.27.1 and replacing deprecated reflection APIs), and changes in CMake and repository structure. In addition, the CHANGELOG was updated to reflect reordering of changelog entries and new items. Overall, these changes aim to improve developer experience, code consistency and reliability of builds and tests.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Update and standardize Jupyter Notebooks
  • Updated notebook content across several notebooks (e.g. Basics, Data Records, Data Transfer, Collections, Collaborative Exercise, and verify notebooks) including reformatting markdown, exercise instructions, and code examples.
  • Removed or revised placeholders and added new exercise instructions.
  • Added a new GitHub workflow to lint Jupyter notebooks with nbQA, flake8, and black.
jupyter_notebooks/0_verify.ipynb
jupyter_notebooks/1_Basics.ipynb
jupyter_notebooks/1_Basics_with_Solutions.ipynb
jupyter_notebooks/2_Data_Records.ipynb
jupyter_notebooks/2_Data_Records_with_Solutions.ipynb
jupyter_notebooks/3_Data_Transfer.ipynb
jupyter_notebooks/3_Data_Transfer_with_Solutions.ipynb
jupyter_notebooks/4_Collections_Queries.ipynb
jupyter_notebooks/5_Collaborative_Exercise.ipynb
.github/workflows/jupyternotebook-lint.yml
jupyter_notebooks/.flake8
Enhance CI/CD and Build Script configurations
  • Updated GitLab build scripts (build_repo_image.yml and build_gcs_image.yml) to include additional file paths (e.g. CMakeLists.txt, cmake directory, repository/server, repository/gridftp) in the change detection.
  • Modified GitHub workflow for format-check to adjust Black formatting arguments (adding --line-length 88) and removed redundant max-line-length parameters from flake8.
  • Added new configuration files for code formatting including a pyproject.toml with Black settings and a root .flake8 file.
.gitlab/build/build_repo_image.yml
.gitlab/build/build_gcs_image.yml
.github/workflows/format-check.yml
pyproject.toml
.flake8
Upgrade dependencies and update library APIs
  • Updated the protobuf dependency in requirements.txt from version 4.21.1 to 5.27.1.
  • Replaced deprecated protobuf reflection API usage in python/datafed_pkg/datafed/Connection.py – eliminated calls to google.protobuf.reflection.MakeClass and ParseMessage, and replaced them with calls to a GetMessageClass helper function.
  • Adjusted the makeMessage function to use GetMessageClass for consistency.
python/datafed_pkg/datafed/Connection.py
python/datafed_pkg/requirements.txt
Update CMake and repository structure
  • Modified repository/CMakeLists.txt to include or exclude relevant subdirectories.
  • Added file changes in cmake// and repository/server//* to reflect new build structure.
  • Updated build scripts to detect changes in the CMake related files.
repository/CMakeLists.txt
cmake/**/*
repository/server/**/*
repository/gridftp/**/*
Revise CHANGELOG to reflect recent fixes and improvements
  • Reordered and modified changelog entries to include a new entry for deprecated method usage and updated dependency versions.
  • Added a new changelog entry for documentation fix in Jupyter notebooks.
CHANGELOG.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @JoshuaSBrown - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider using a tool like pre-commit to run linters and formatters before committing.
Here's what I looked at during the review
  • 🟡 General issues: 3 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

"metadata": {},
"outputs": [],
"source": [
"df_api.collectionView('projshare', context='p/trn001)"
Copy link

Choose a reason for hiding this comment

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

issue (bug_risk): Missing closing quote in the context argument may cause a syntax error.

It appears that the context string is not properly closed. Please verify that the intended string (likely 'p/trn001') is correctly delimited.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please add this to issue #1300

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please add this in an issue and link to #1300

Copy link

Choose a reason for hiding this comment

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

I have created an issue for your comment: #1308

Comment on lines +116 to +117
- **References Tab**:
This section lists locations where the schema is referenced within DataFed.
Copy link

Choose a reason for hiding this comment

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

issue (typo): Typo: "section" should be "tab"

Under the "References Tab" section, it currently says, "This section lists locations...". It should be "This tab lists locations...".

Suggested change
- **References Tab**:
This section lists locations where the schema is referenced within DataFed.
- **References Tab**:
- This tab lists locations where the schema is referenced within DataFed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please add this to issue #1300

Copy link

Choose a reason for hiding this comment

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

I have created an issue for your comment: #1303

22. [1255] - Fixes bug, in libauthz with log_path var not being read in from config file.
23. [1268] - Bug fixes unbound variable in foxx entrypoint file.
24. [1269] - Update Arangodb to 3.12.4
25. [1288] - Bug Jupyternote books in documentation were fixed.
Copy link

Choose a reason for hiding this comment

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

issue (typo): Typo: "Jupyternote books" should be "Jupyter notebooks"

Suggested change
25. [1288] - Bug Jupyternote books in documentation were fixed.
25. [1288] - Bug Jupyter notebooks in documentation were fixed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please add this to issue #1300

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please add this in an issue and link to #1300

Copy link

Choose a reason for hiding this comment

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

I have created an issue for your comment: #1307

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.

2 participants