Skip to content

tooling: Add Github release manager#17741

Merged
htuch merged 2 commits intoenvoyproxy:mainfrom
phlax:tooling-gh-manager
Aug 25, 2021
Merged

tooling: Add Github release manager#17741
htuch merged 2 commits intoenvoyproxy:mainfrom
phlax:tooling-gh-manager

Conversation

@phlax
Copy link
Member

@phlax phlax commented Aug 17, 2021

Commit Message: tooling: Add Github release manager
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Aug 17, 2021
@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #17741 was opened by phlax.

see: more, trace.

@phlax phlax marked this pull request as draft August 17, 2021 13:09
@phlax
Copy link
Member Author

phlax commented Aug 17, 2021

@phlax phlax force-pushed the tooling-gh-manager branch 10 times, most recently from 24e33aa to 7c14fbd Compare August 20, 2021 16:58
@phlax phlax marked this pull request as ready for review August 20, 2021 17:23
@phlax phlax changed the title [WIP] tooling: Add Github release manager tooling: Add Github release manager Aug 20, 2021
@phlax phlax requested a review from htuch August 20, 2021 17:24
@phlax phlax changed the title tooling: Add Github release manager [WIP] tooling: Add Github release manager Aug 20, 2021
@phlax phlax marked this pull request as draft August 20, 2021 20:24
@phlax
Copy link
Member Author

phlax commented Aug 20, 2021

briefly reWIPping while i add paging for addittional release versions

@phlax phlax force-pushed the tooling-gh-manager branch from 7c14fbd to 9ba761e Compare August 23, 2021 15:10
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax phlax force-pushed the tooling-gh-manager branch from 9ba761e to d5c70e2 Compare August 23, 2021 16:37
@phlax phlax changed the title [WIP] tooling: Add Github release manager tooling: Add Github release manager Aug 23, 2021
@phlax phlax marked this pull request as ready for review August 23, 2021 20:33

from tools.github.release.abstract import AGithubRelease, AGithubReleaseManager
from tools.github.release.exceptions import GithubReleaseError
# from tools.github.release.release import GithubRelease
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Member Author

Choose a reason for hiding this comment

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

in order to land them separately i commented out the interdependent bits in the tests and class

return pathlib.Path(self._path)

@async_property
async def latest(self) -> Dict[str, packaging.version.Version]:
Copy link
Member

Choose a reason for hiding this comment

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

Is this entire class and manager just a very elaborate way to get at latest version? The contrast is some of what we do in https://github.com/envoyproxy/envoy/blob/main/tools/dependency/release_dates.py. That's also not very concise.

I thought based on the comments in the abstract interface, this would actually be doing some artifact fetch/push.

Copy link
Member Author

Choose a reason for hiding this comment

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

This class does quite a lot

Overall the tool is structured broadly into 3 classes

GithubRelease - CRUD operations
GithubReleaseAssets - upload/download of assets

and this one GithubReleaseManager - which:

  • provides logging and warn/error handling
  • holds the github connection and http client session
  • iterates releases and creates a dictionary of latest major/minor versions
  • version formatting/parsing (ie v0.0.1 -> packaging.Version("0.0.1"))
  • provides a factory for GithubRelease objects

its my intention to rewrite the release dates util at some point to make use of a Runner, improve logging/error handling, add tests etc. In that event the release dates util can almost certainly make use of this lib

I thought based on the comments in the abstract interface, this would actually be doing some artifact fetch/push.

if you look through the interfaces again, i think you will see that this is an implementation of the manager class specified there

artifact fetch/push comes after

Signed-off-by: Ryan Northey <ryan@synca.io>
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks.

@htuch htuch merged commit 00b5399 into envoyproxy:main Aug 25, 2021
mpuncel added a commit to mpuncel/envoy that referenced this pull request Aug 25, 2021
* main:
  config: fix dfp config validation (envoyproxy#17835)
  docs: updating where meetings are uploaded (envoyproxy#17832)
  h2: moving a comment (envoyproxy#17846)
  quiche: early fail listener config with both quic and connection_balencer (envoyproxy#17834)
  dns: configuring a basic key value store to persist to disk (envoyproxy#17745)
  quic: fix receiving STOP_SENDING (envoyproxy#17815)
  tooling: Add Github release manager (envoyproxy#17741)
  tooling: Use upstream pytest-patches (envoyproxy#17809)
  Remove `hidden_envoy_deprecated_use_http2` (envoyproxy#17805)
  kafka: produce request for mesh-filter (envoyproxy#17818)

Signed-off-by: Michael Puncel <mpuncel@squareup.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deps Approval required for changes to Envoy's external dependencies

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants