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

Allow multiple query combined with versionfilter to json source #944

Merged
merged 7 commits into from
Oct 24, 2022

Conversation

olblak
Copy link
Member

@olblak olblak commented Oct 19, 2022

Signed-off-by: Olblak me@olblak.com

As discussed on #942 (comment)
I realized that it would be super usefull to be able to do a multiple query combined with versionfilter on the json resource

This pullrequest allows the following manifest to work

sources:
  semverVersion:
    kind: json
    name: Get latest version from remote json file
    spec:
      file: https://api.github.com/repos/updatecli/updatecli/releases
      key: ".[*].tag_name"
      multiple: true
      versionfilter:
        kind: semver
        pattern: "v0.35"

Example

+++++++++++
+ PREPARE +
+++++++++++

Loading Pipeline "/tmp/updatelci.yaml"

SCM repository retrieved: 0


++++++++++++++++++
+ AUTO DISCOVERY +
++++++++++++++++++



++++++++++++
+ PIPELINE +
++++++++++++



##################
# UPDATELCI.YAML #
##################


SOURCES
=======

semverVersion
-------------
Searching for version matching pattern "v0.35"
✔ Value "v0.35.1", found in file "https://api.github.com/repos/updatecli/updatecli/releases", for key ".[*].tag_name"'

=============================

REPORTS:


- UPDATELCI.YAML:
	Source:
		✔ [semverVersion] Get latest version from remote json file (kind: json)

Run Summary
===========
Pipeline(s) run:
  * Changed:	0
  * Failed:	0
  * Skipped:	1
  * Succeeded:	0
  * Total:	1

Test

To test this pull request, you can run the following commands:

go build -o bin/updatecli .
./bin/updatecli --config manifest_mentioned_here.yaml
go test

Additional Information

Tradeoff

Potential improvement

I'll try to find some time to open a similar pullrquest for toml and csv as they also use Dasel
I need to investigate for the xml resource but it should be possible
Unfortunately, it won't work for the yaml resource as it doesn't support multiple query

@olblak olblak added enhancement New feature or request resource-json labels Oct 19, 2022
Signed-off-by: Olblak <me@olblak.com>
@dduportal
Copy link
Contributor

Like in #945 (comment), it seems that this would be a broader change as it implies having a multi-valued source.

How is the order managed?

@dduportal
Copy link
Contributor

Can you update the PR description to show the result? The example is not clear on what is it doing exactly?

@olblak
Copy link
Member Author

olblak commented Oct 20, 2022

Like in #945 (comment), it seems that this would be a broader change as it implies having a multi-valued source.

The source output doesn't change as we still return one single value. Here versionfilter works the same way than for plugins

  • githubrelease
  • giteaTag
  • gittag
  • dockerimage

Basically we retrieve a list of information and we delegate the responsability to the versionfilter to identify the value we are looking for

So this pullrequest introduced two parameters

  1. multiple: It's used to describe that we are looking for multiple values from the json query and not only one. Because .[*].tag_name is a valid json key identifer
  2. versionfilter: It's used to filter the result from the multiple query

In the context of a source, both must be used together

@olblak
Copy link
Member Author

olblak commented Oct 20, 2022

Can you update the PR description to show the result? The example is not clear on what is it doing exactly?

I added an example in the PR body

@dduportal
Copy link
Contributor

🤔 the boolean multiple feels very weird. It's not clear what it does unless you really know.

WDYT about having either key (with a strict litteral match) or query (that implies multiple results)?

@olblak
Copy link
Member Author

olblak commented Oct 20, 2022

I am open to change if it makes things clearier. Ideally I would like to use the same wording for csv/toml/xml/json/yaml
Maybe @hervelemeur has some opinion too

olblak and others added 3 commits October 23, 2022 09:35
@olblak
Copy link
Member Author

olblak commented Oct 24, 2022

Before merging I need to correctly deprecate the key multiple previously used for conditoin and target

@olblak olblak merged commit 1aeeca4 into updatecli:main Oct 24, 2022
@olblak olblak added this to the 0.37.0 milestone Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request resource-json
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants