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 update-specs updatcli workflow #1745

Merged
merged 3 commits into from
Feb 9, 2023

Conversation

reakaleek
Copy link
Member

@reakaleek reakaleek commented Feb 8, 2023

What does this pull request do?

Moves the update specs workflow from apm-server to the consumer using updatecli to have a "dependabot"-like experience.

How to test

GITHUB_TOKEN=<token>\
GIT_USER=<username> \
GIT_EMAIL=j<email> \
updatecli diff -c .ci/update-specs.yml

Related issues

@github-actions github-actions bot added agent-python community Issues opened by the community triage Issues awaiting triage labels Feb 8, 2023
@reakaleek reakaleek requested review from a team February 8, 2023 12:06
@reakaleek reakaleek self-assigned this Feb 8, 2023
@reakaleek reakaleek marked this pull request as ready for review February 8, 2023 12:07
@reakaleek reakaleek requested a review from axw February 8, 2023 12:08
Comment on lines +23 to +42
error.json:
kind: file
spec:
file: https://raw.githubusercontent.com/elastic/apm-data/main/input/elasticapm/docs/spec/v2/error.json
metadata.json:
kind: file
spec:
file: https://raw.githubusercontent.com/elastic/apm-data/main/input/elasticapm/docs/spec/v2/metadata.json
metricset.json:
kind: file
spec:
file: https://raw.githubusercontent.com/elastic/apm-data/main/input/elasticapm/docs/spec/v2/metricset.json
span.json:
kind: file
spec:
file: https://raw.githubusercontent.com/elastic/apm-data/main/input/elasticapm/docs/spec/v2/span.json
transaction.json:
kind: file
spec:
file: https://raw.githubusercontent.com/elastic/apm-data/main/input/elasticapm/docs/spec/v2/transaction.json
Copy link
Member Author

Choose a reason for hiding this comment

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

@axw
Is it okay for this to be a "static list" of files, or do we need something more dynamic here that considers file additions/deletions?

Copy link
Member

Choose a reason for hiding this comment

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

What are our other options? The set of files won't change often, but they might still change. I think we could go ahead with this, and look at bundling the schema documents into a Compound Schema Document: http://json-schema.org/understanding-json-schema/structuring.html#bundling

Copy link
Member

Choose a reason for hiding this comment

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

Opened elastic/apm-data#15 to improve this

Copy link
Member Author

@reakaleek reakaleek Feb 9, 2023

Choose a reason for hiding this comment

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

Another option could be a shell script that does the cloning and copying of the files. Which feels a bit hacky in the updatecli way, but is certainly possible.

Regardless, a compound schema sounds really good! This will also account for deletions.

I actually encountered a case where log.json was not removed (Also in the current implementation). (See elastic/apm-agent-nodejs#3153 (review)) A single file would fix this behaviour.

P.S.

From what I've seen, something like rsync --delete, to account for deletions, probably won't work seemlessly either.. because in some repos the schemas are in the same folder with other schemas.

@reakaleek reakaleek merged commit 37b1f37 into elastic:main Feb 9, 2023
@reakaleek reakaleek deleted the feature/update-specs-action branch February 9, 2023 08:46
beniwohli pushed a commit to beniwohli/apm-agent-python that referenced this pull request Feb 16, 2023
* Add update-specs updatcli workflow

* Fix paths
basepi added a commit that referenced this pull request Feb 16, 2023
* handle case when no span is created in GRPC client interceptor

closes #1739

* Migrate Jenkinsfile 2 GH Actions Workflow (#1731)

* Migrate Jenkinsfile to GH Actions

* Generic naming

* Upload junit test and coverage reports on success or on failure

* Better naming

* Remove cron from packages workflow

* Rename all occurences of WEBFRAMEWORK to FRAMEWORK

* Add a warning about BaseHTTPMiddleware to Starlette docs (#1735)

* Add a warning about BaseHTTPMiddleware to Starlette docs

Also switch to using get_client() and make the docs simpler for
environment variable configuration.

* CHANGELOG

* Change `server_url` default to avoid ipv6 ambiguity (#1744)

* Change `server_url` default to avoid ipv6 ambiguity

* Fix failing test and another minor doc fix

* Add service.agent.activation_method to metadata (#1743)

* Add activation_method to metadata

* Add test

* Move activation_method default above start_threads

* CHANGELOG

* Fix nightly scheduled test (#1747)

* Split matrix items into chunks

to bypass the 256 limit of matrix items in github actions

* cleanup

* Add comments

* Add update-specs updatcli workflow (#1745)

* Add update-specs updatcli workflow

* Fix paths

* Required Status Check (#1749)

* Create single status check that can be set as required

* Let windows test runs exit with the correct exit code

* Set the exit code

* Formatting

* fix

* More readable jq query

* Set status check to success in case it's an only-docs PR (#1753)

* Set status check to success in case it's a only-docs PR

* Set permissions

* fix path patterns

* Add comments

* Update badge (#1752)

* Add dynamic config tag to more supported options (#1750)

* Fix sha source (#1754)

* update changelog

---------

Co-authored-by: Jan Calanog <[email protected]>
Co-authored-by: Colton Myers <[email protected]>
Co-authored-by: Jan Calanog <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-python community Issues opened by the community triage Issues awaiting triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants