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

Add hash for OR-Tools, remove unneccessary pip repositories call #372

Merged
merged 1 commit into from
Oct 3, 2022

Conversation

jamesreprise
Copy link
Contributor

@jamesreprise jamesreprise commented Oct 3, 2022

What is the goal of this PR?

Currently, there are a number of warnings output by Bazel upon syncing dependencies. These four warnings provide some recommendations as to how we can improve the state of the repository. Additionally, fixing the problems associated with warnings is good practice as it prevents developers from becoming used to warnings and potentially missing something important. Case in point: if someone were to compromise the google/or-tools repository and replace the releases we download with compromised packages, Bazel wouldn't know.

We are currently missing a hash for our http_archive downloads for google_or_tools_*. This reduces hermeticity.

We currently call pip_repositories, despite the fact that it is a deprecated method. See the method body:

def pip_repositories():
    """
    Obsolete macro to pull in dependencies needed to use the pip_import rule.

    Deprecated:
        the pip_repositories rule is obsolete. It is not used by pip_install.
    """

    # buildifier: disable=print
    print("DEPRECATED: the pip_repositories rule has been replaced with pip_install, please see rules_python 0.1 release notes")

What are the changes implemented in this PR?

  • We have added a sha256 argument to all of our google_or_tools_* http_archive calls, increasing hermeticity.
  • We have removed our call to pip_repositories.

@grabl
Copy link
Contributor

grabl commented Oct 3, 2022

PR Review Checklist

Do not edit the content of this comment. The PR reviewer should simply update this comment by ticking each review item below, as they get completed.


Code

  • Packages, classes, and methods have a single domain of responsibility.
  • Packages, classes, and methods are grouped into cohesive and consistent domain model.
  • The code is canonical and the minimum required to achieve the goal.
  • Modules, libraries, and APIs are easy to use, robust (foolproof and not errorprone), and tested.
  • Logic and naming has clear narrative that communicates the accurate intent and responsibility of each module (e.g. method, class, etc.).
  • The code is algorithmically efficient and scalable for the whole application.

Architecture

  • Any required refactoring is completed, and the architecture does not introduce technical debt incidentally.
  • Any required build and release automations are updated and/or implemented.
  • Any new components follows a consistent style with respect to the pre-existing codebase.
  • The architecture intuitively reflects the application domain, and is easy to understand.
  • The architecture has a well-defined hierarchy of encapsulated components.
  • The architecture is extensible and scalable.

@grabl grabl removed the request for review from alexjpwalker October 3, 2022 09:04
@jamesreprise jamesreprise added type: refactor priority: medium dependencies Pull requests that update a dependency file labels Oct 3, 2022
@jamesreprise jamesreprise marked this pull request as ready for review October 3, 2022 09:05
@jamesreprise
Copy link
Contributor Author

@jamesreprise jamesreprise merged commit 8d22651 into typedb:master Oct 3, 2022
jamesreprise added a commit to typedb/typedb-driver-python that referenced this pull request Oct 3, 2022
## What is the goal of this PR?

We remove an unneccessary call of `pip_repositories` despite it being a
deprecated function from `rules_python`, including in our own patched
version. Additionally, this cleans up our bazel command output. See the
output message below.

```
DEBUG: /private/var/tmp/[...]/external/rules_python/python/pip.bzl:228:10: 
DEPRECATED: the pip_repositories rule has been replaced with pip_install, please see rules_python 0.1 release notes
```

See this dependencies PR for more information:
- typedb/typedb-dependencies#372

## What are the changes implemented in this PR?

We remove the command that loads the function as well as the call to the
function.
flyingsilverfin pushed a commit to typedb/typedb-ml that referenced this pull request Oct 4, 2022
## What is the goal of this PR?

We remove an unneccessary call of `pip_repositories` despite it being a deprecated function from `rules_python`, including in our own patched version. Additionally, this cleans up our bazel command output. See the output message below.

```
DEBUG: /private/var/tmp/[...]/external/rules_python/python/pip.bzl:228:10: 
DEPRECATED: the pip_repositories rule has been replaced with pip_install, please see rules_python 0.1 release notes
```

See this dependencies PR for more information:
- typedb/typedb-dependencies#372

## What are the changes implemented in this PR?

We remove the command that loads the function as well as the call to the function.
@jamesreprise jamesreprise deleted the cleanup branch December 5, 2022 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file priority: medium type: refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants