这是indexloc提供的服务，不要输入任何密码
Skip to content

read version from env var during server build time (close #1398) #1897

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Apr 11, 2019
Merged

read version from env var during server build time (close #1398) #1897

merged 11 commits into from
Apr 11, 2019

Conversation

@shahidhk
Copy link
Member

@shahidhk shahidhk commented Mar 28, 2019 •
edited
Loading

Description

This PR modifies the server code to read the version from an env var VERSION during build time. Earlier it was done by execution scripts/get-version.sh. Also adds build and exec rules to the makefile.

If the env var is not set, it defaults to the output of scripts/get-version.sh.

This makes it easier to build server with custom version during dev - especially when CLI/Server compatibility needs to be tested/worked upon.

Affected components

  • Server
  • Build System

Related Issues

close #1398

Solution and Design

getValFromEnv :: String -> TH.Q TH.Exp function is added to Hasura.Server.Utils module and this function is called from Hasura.Server.Version module as version = $(getValFromEnv "VERSION")

Steps to test and verify

Build the server with a custom version:

VERSION=my-custom-version make build

Run the server:

HASURA_GRAPHQL_DATABASE_URL=postgres://postgres:@localhost:5432/postgres make exec

Hit the /v1/version endpoint:

curl http://localhost:8080/v1/version   
{"version":"my-custom-version"}

Limitations, known bugs & workarounds

NA

@shahidhk shahidhk requested a review from 0x777 as a code owner March 28, 2019 12:09
@shahidhk shahidhk added c/server Related to server c/build-system Related to the build-release system labels Mar 28, 2019
@netlify
Copy link

netlify bot commented Mar 28, 2019

Deploy preview for hasura-docs ready!

Built with commit 6f08de2

https://deploy-preview-1897--hasura-docs.netlify.com

@netlify
Copy link

netlify bot commented Mar 28, 2019 •
edited
Loading

Deploy preview for hasura-docs ready!

Built with commit ae0a6a4

https://deploy-preview-1897--hasura-docs.netlify.com

@shahidhk shahidhk added the s/do-not-merge Do not merge this pull request to master label Mar 28, 2019
@hasura-bot
Copy link
Contributor

hasura-bot commented Mar 28, 2019

Review app for commit 4eb0543 deployed to Heroku: https://hge-ci-pull-1897.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1897-4eb0543

@shahidhk shahidhk added s/ok-to-merge Status: This pull request can be merged to master and removed s/do-not-merge Do not merge this pull request to master labels Mar 28, 2019
ecthiender
ecthiender previously approved these changes Mar 28, 2019
View reviewed changes
Copy link
Contributor

@ecthiender ecthiender left a comment

Choose a reason for hiding this comment

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

LGTM

0x777
0x777 previously approved these changes Apr 5, 2019
View reviewed changes
@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 5, 2019

Review app for commit b4414e0 deployed to Heroku: https://hge-ci-pull-1897.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1897-b4414e0

@shahidhk
Copy link
Member Author

shahidhk commented Apr 5, 2019 •
edited
Loading

One thing to note is that the build will throw an error if env var VERSION is not set. This needs to be added to the contributing guide and instructions should be given with make instead of stack directly.

Do not merge the PR until this is added to contributing guide.

The script is executed if env var is missing.

@shahidhk shahidhk dismissed stale reviews from 0x777 and ecthiender via 60fdcf9 April 8, 2019 06:47
@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 8, 2019

Review app for commit 60fdcf9 deployed to Heroku: https://hge-ci-pull-1897.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1897-60fdcf9

nizar-m
nizar-m approved these changes Apr 8, 2019
View reviewed changes
Copy link
Contributor

@nizar-m nizar-m left a comment

Choose a reason for hiding this comment

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

LGTM

@0x777 0x777 merged commit de24cfd into hasura:master Apr 11, 2019
@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 11, 2019

Review app https://hge-ci-pull-1897.herokuapp.com is deleted

@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 11, 2019

Review app for commit ae0a6a4 deployed to Heroku: https://hge-ci-pull-1897.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1897-ae0a6a4

polRk pushed a commit to polRk/graphql-engine that referenced this pull request Feb 12, 2020
@shahidhk @polRk
read version from env var during server build time (close hasura#1398) (
4923509 
hasura#1897)

* read version from env var at build time (close hasura#1398)

* remove un-used imports, edit makefile

* edit makefile to add new targets and export variables

* only export VERSION in makefile

* read version by executing the script if env var is absent
hasura-bot pushed a commit that referenced this pull request May 7, 2025
@danieljharvey @hasura-bot
Fix relational query / NDC diff tests (#1897)
97e2fd1 
<!-- The PR description should answer 2 important questions: -->

### What

The SQL tests checked whether skipping relational pushdown gave us the
same result as using the NDC / DataFusion route. They only worked
sometimes, now they should run always. As a result, more tests are
running and we need to fix a few things.

1) the custom connector doesn't like missing fields, changes to use an
explicit `null`.
2) we add an ability to explicitly skip these diff tests, as a few
places they don't work.

V3_GIT_ORIGIN_REV_ID: 3a720d09e37d497ea21e42f782ad651106113e0c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0x777 0x777 0x777 approved these changes

@nizar-m nizar-m nizar-m approved these changes

+1 more reviewer

@ecthiender ecthiender ecthiender left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c/build-system Related to the build-release system c/server Related to server s/ok-to-merge Status: This pull request can be merged to master

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

read version from env var during build time

5 participants

@shahidhk @hasura-bot @ecthiender @0x777 @nizar-m