Skip to content
This repository has been archived by the owner on Jun 5, 2023. It is now read-only.

Sync code from release instead of git repo #96

Merged
merged 2 commits into from Mar 8, 2017
Merged

Sync code from release instead of git repo #96

merged 2 commits into from Mar 8, 2017

Conversation

carise
Copy link
Contributor

@carise carise commented Mar 8, 2017

No description provided.

@@ -54,7 +55,7 @@ resources:
scanner-bucket: SCANNER_BUCKET
database-name: forseti_security
organization-id: YOUR_ORG_ID
src-path: https://github.com/GoogleCloudPlatform/forseti-security
src-path: https://github.com/GoogleCloudPlatform/forseti-security/archive/v1.0.tar.gz
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can the src-path be dynamic based on the version in the properties. Which could be set to HEAD to get current repo, or a specific version to get that release?

Slightly off-topic, it would be nice if the release included the compiled protos so protoc wouldn't be required when running a release version.

Copy link
Contributor Author

@carise carise Mar 8, 2017

Choose a reason for hiding this comment

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

I'll revisit the dynamic src-path and see if I can get that working with another PR. I'll also see if I can get the compiled protos in the release, in that PR.

@@ -128,8 +129,8 @@ def GenerateConfig(context):
pip install --upgrade setuptools

cd $USER_HOME
git clone {}
cd forseti-security
wget -qO- {} | tar xvz
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be cleaner if the format() string used named identifiers for the variables instead of relying on positional arguments.

So something like:

startup_script_config = {'forseti-version': context.properties['forseti-version'], ...}
"""
...
wget -q0- {forseti-version} | tar xvz
...
""".format(**startup_script_config)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll create a followup PR for that.

@carise carise merged commit 2fba3ba into master Mar 8, 2017
@carise carise deleted the dm-release branch March 8, 2017 16:34
@blueandgold
Copy link
Contributor

Adding cla: yes label, as the PR author is a googler at the time of this PR was created.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants