search

anz-bank / sysl

Sysl (pronounced "sizzle") is a system specification language
https://sysl.io
Apache License 2.0
122 stars 42 forks source link

Support importing sysl modules via GitHub API #993

Closed ChloePlanet closed 4 years ago

ChloePlanet commented 4 years ago

Description:

GitHub API based sysl modules

$ export SYSL_MODULES=github
$ export SYSL_GITHUB_TOKEN=**************

There's no dependency list and lock file required under the working directory. Which means if there are the same importing statements in multiple files, developers need to update all of them when they want to upgrade all of them. This is not a problem for sysl-catalog for now as sysl-catalog is supposed to import every dependent file once (so only in one file). If the dependency maintaining becomes an issue in the future, one approach is to add an optional lock file, and change the behaviour of import //github.com/anz-bank/sysl-go that it matches the version indicating in lock file instead of fetching the latest version every time.

Note

The behaviour of go modules based sysl modules is a bit different from GitHub API based sysl modules:

$ export SYSL_MODULES=gomod // or SYSL_MODULES=on

Changes:

Checklist:

codecov[bot] commented 4 years ago

Codecov Report

Merging #993 into master will increase coverage by 1.09%. The diff coverage is 83.30%.

@@            Coverage Diff             @@
##           master     #993      +/-   ##
==========================================
+ Coverage   83.35%   84.44%   +1.09%     
==========================================
  Files          73       77       +4     
  Lines       10609    10697      +88     
==========================================
+ Hits         8843     9033     +190     
+ Misses       1432     1344      -88     
+ Partials      334      320      -14
Impacted Files Coverage Δ
pkg/mod/github.go 54.62% <54.62%> (ø)
pkg/importer/importer.go 63.15% <63.15%> (ø)
pkg/mermaid/datamodeldiagram/datamodeldiagram.go 61.90% <68.42%> (+3.33%) :arrow_up:
pkg/parse/parse.go 85.34% <73.07%> (+1.37%) :arrow_up:
pkg/mod/module.go 82.85% <78.57%> (+17.14%) :arrow_up:
pkg/mod/gomodules.go 74.39% <79.16%> (ø)
...g/mermaid/integrationdiagram/integrationdiagram.go 83.19% <82.45%> (-1.19%) :arrow_down:
pkg/mod/fs.go 71.92% <83.33%> (+2.11%) :arrow_up:
pkg/importer/openapi.go 86.12% <85.45%> (-3.32%) :arrow_down:
pkg/parse/listener_impl.go 89.62% <86.48%> (+0.11%) :arrow_up:
... and 28 more
ChloePlanet commented 4 years ago

Also, it's likely we'll hit rate-limiting for unauthenticated requests (60/h) so we might want to add handling of the rate-limiting error message fairly early on https://github.com/google/go-github#rate-limiting

Good catch!

anzdaddy commented 4 years ago

The rate limiting could be a showstopper if it hits the API too hard. Is this likely to be a problem with the current caching approach? What's involved in using an API key and what are the downsides?

anzdaddy commented 4 years ago

Do we need per-import versioning? I thought all the use cases we discussed could be solved by other means.

ChloePlanet commented 4 years ago

The rate limiting could be a showstopper if it hits the API too hard. Is this likely to be a problem with the current caching approach? What's involved in using an API key and what are the downsides?

Added an envvar SYSL_GITHUB_TOKEN to get user's GitHub PAT. And using secrets.GITHUB_TOKEN in CI as the access token.

Do we need per-import versioning? I thought all the use cases we discussed could be solved by other means.

We don't need it in go modules based sysl modules as it could be specified in go.mod. But it's needed in GitHub API based sysl modules since import //github.com/anz-bank/sysl-go always fetches the latest version and there's no lock file to save the version.