Skip to content

feat: Enable time substitution in spec templates#20399

Merged
kodiakhq[bot] merged 3 commits intomainfrom
feat/env-subsitution
Mar 21, 2025
Merged

feat: Enable time substitution in spec templates#20399
kodiakhq[bot] merged 3 commits intomainfrom
feat/env-subsitution

Conversation

@przste-go
Copy link
Copy Markdown

@przste-go przste-go commented Mar 19, 2025 •
edited by erezrokah
Loading

Summary

⚠️ If you're contributing to a plugin please read this section of the contribution guidelines 🧑‍🎓 before submitting this PR ⚠️

This PR aims to enable relative time support in all spec templates used is CQ platform without additional type overriding on plugin side. I.e following aws table options can be automatically substituted using yaml parser.

      aws_cloudtrail_events:
        lookup_events:
          - start_time: ${time:1 days ago}
            lookup_attributes:
              - attribute_key:   EventName
                attribute_value: RunInstances

BEGIN_COMMIT_OVERRIDE
feat: Enable time substitution in spec templates (#20399). See our docs for more information.

END_COMMIT_OVERRIDE

@przste-go przste-go marked this pull request as ready for review March 19, 2025 08:25
@przste-go przste-go requested review from a team and jon-s58 March 19, 2025 08:25
@przste-go przste-go force-pushed the feat/env-subsitution branch from 4c20d02 to 10c05d3 Compare March 19, 2025 08:25
erezrokah
erezrokah reviewed Mar 20, 2025
View reviewed changes
cli/internal/specs/v0/testdata/time_substitution.yml Outdated
tables: [test]
spec:
# Specifying non-relative time for easier testing
table_options: ${time:2025-01-01}
Copy link
Copy Markdown
Member

@erezrokah erezrokah Mar 20, 2025

Choose a reason for hiding this comment

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

Can we use a real example for a table in the AWS plugin? So people don't get confused and think this is something that would work

Copy link
Copy Markdown
Author

@przste-go przste-go Mar 20, 2025

Choose a reason for hiding this comment

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

Replaced with real table_options 👍

@murarustefaan
Copy link
Copy Markdown
Member

murarustefaan commented Mar 20, 2025

I don't think this needs to be marked as a breaking change, but is rather a feature people can opt-in to use and it should not affect anything else otherwise. Am I wrong?

@przste-go
Copy link
Copy Markdown
Author

przste-go commented Mar 20, 2025

I don't think this needs to be marked as a breaking change, but is rather a feature people can opt-in to use and it should not affect anything else otherwise. Am I wrong?

There is rather small chance that someone named their env variable ${time:} so if you think it's allright to push it as minor then let's do it :D

@murarustefaan
Copy link
Copy Markdown
Member

murarustefaan commented Mar 20, 2025

@przste-go while permitted by some implementations, they are not allowed or supported by the RFC.

https://pubs.opengroup.org/onlinepubs/9799919799/

Environment variable names used by the utilities in the Shell and Utilities volume of POSIX.1-2024 consist solely of uppercase letters, digits, and the ('_') from the characters defined in Portable Character Set and do not begin with a digit.

I don't think it's worth creating this as a breaking change for something that

  1. is not used the way it should be and
  2. has a very unlikely chance of happening.

Realistically, I tried to create these ENV's and I was not able to.

bash-3.2$ export test:=test
bash: export: `test:=test': not a valid identifier
bash-3.2$ export "test:=test"
bash: export: `test:=test': not a valid identifier

@przste-go
Copy link
Copy Markdown
Author

przste-go commented Mar 20, 2025

sounds fair

@przste-go przste-go changed the title feat!: Enable time substitution in spec templates feat: Enable time substitution in spec templates Mar 20, 2025
erezrokah
erezrokah approved these changes Mar 20, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

Looks good, made the changelog a bit more descriptive via a commit override and a link to our docs (to be created)

@przste-go przste-go added the automerge Automatically merge once required checks pass label Mar 21, 2025
@kodiakhq kodiakhq bot merged commit b46842d into main Mar 21, 2025
17 checks passed
@kodiakhq kodiakhq bot deleted the feat/env-subsitution branch March 21, 2025 11:27
@cq-bot cq-bot mentioned this pull request Mar 21, 2025
Merged
kodiakhq bot pushed a commit that referenced this pull request Mar 24, 2025
@cq-bot
chore(main): Release cli v6.17.0 (#20402)
52a784f 
🤖 I have created a release *beep* *boop*
---


## [6.17.0](cli-v6.16.0...cli-v6.17.0) (2025-03-24)


### Features

* Enable time substitution in spec templates ([#20399](#20399)). See our [docs](https://docs.cloudquery.io/docs/advanced-topics/environment-variable-substitution#time-variable-substitution-example) for more information. ([b46842d](b46842d))


### Bug Fixes

* **deps:** Update module github.com/apache/arrow-go/v18 to v18.2.0 ([#20410](#20410)) ([ee081fb](ee081fb))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@akash1810
Copy link
Copy Markdown
Contributor

akash1810 commented Apr 25, 2025

I love this change! How might I create a relative date of format YYYY-MM-DD? This format is required for the aws_costexplorer_cost_custom table's TimePeriod option.

@przste-go
Copy link
Copy Markdown
Author

przste-go commented Apr 25, 2025

Hi @akash1810 we have that feature planned for current development cycle, so it should be publicly available in upcoming weeks 🚀

@przste-go przste-go mentioned this pull request Apr 28, 2025
Merged
kodiakhq bot pushed a commit that referenced this pull request Apr 28, 2025
feat: Support different formats for time variables (#20626)
4242ce5 
#### Summary

⚠️ **If you're contributing to a plugin please read this section of the [contribution guidelines](https://github.com/cloudquery/cloudquery/blob/main/CONTRIBUTING.md#open-core-vs-open-source) 🧑‍🎓 before submitting this PR** ⚠️

This PR follows changes introduced in #20399. In enables formatting substituted times using '|' operator according to golang time layout spec https://go.dev/src/time/format.go.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erezrokah erezrokah erezrokah approved these changes

@jon-s58 jon-s58 Awaiting requested review from jon-s58

Assignees

No one assigned

Labels

area/cli automerge Automatically merge once required checks pass

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@przste-go @murarustefaan @akash1810 @erezrokah @cq-bot