FEATURE:TMA-1610 - Update metric for client localization and bump LCM version by sangtm · Pull Request #1800 · gooddata/gooddata-ruby

        params = params.to_hash
        if params.key?('aws_client')
-          GoodData.logger.info('Setting up AWS-S3 connection')
+          GoodData.logger.warn('The input_source: aws_client is predicated. Please use s3_client instead.')


do not log this warning message

Removed it, thanks

danh-ung · 2021-09-01T10:03:57Z

+        raise 'Unable to connect to AWS. Parameter "accessKey" is missing' if params['s3_client']['accessKey'].blank?
+        raise 'Unable to connect to AWS. Parameter "secretKey" is missing' if params['s3_client']['secretKey'].blank?
+
+        params['aws_client'] = {}


aws_client is deprecated, we should rewrite it opposite to s3_client instea.

Updated to use s3_client instead. Thanks

ghost · 2021-09-02T11:37:41Z

Build in pipeline check aborted.

ghost · 2021-09-02T11:58:44Z

Build succeeded (check pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-oss-check/853/ : SUCCESS in 6s
https://jenkins-ii.intgdc.com/job/gooddata-ruby/job/gooddata-ruby-lcm-build-pipeline/752/ : SUCCESS in 20m 45s

ghost · 2021-09-03T10:29:35Z

Build in pipeline check aborted.

ghost · 2021-09-03T10:31:25Z

Build in pipeline check aborted.

ghost · 2021-09-03T10:51:51Z

Build succeeded (check pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-oss-check/856/ : SUCCESS in 6s
https://jenkins-ii.intgdc.com/job/gooddata-ruby/job/gooddata-ruby-lcm-build-pipeline/755/ : SUCCESS in 20m 16s

ghost · 2021-09-06T13:19:45Z

Build succeeded (check pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-oss-check/857/ : SUCCESS in 7s
https://jenkins-ii.intgdc.com/job/gooddata-ruby/job/gooddata-ruby-lcm-build-pipeline/756/ : SUCCESS in 49m 56s

danh-ung · 2021-09-08T11:22:20Z

+      def get_s3_config(params)
+        s3_config = {}
+        if params.key?('aws_client')
+          raise 'Unable to connect to AWS. Parameter "aws_client" seems to be empty' unless params['aws_client']


Is this redundant? We already have condition if params.key?('aws_client') ?

This error happen when the key aws_client is exist but params['aws_client'] is empty. This is old code acctually. Thank you.

danh-ung · 2021-09-08T12:26:50Z

+        def load_metric_data(params)
+          metric_input_source = validate_input_source(params.input_source)
+          if metric_input_source.nil?
+            GoodData.logger.info('No metric config or it is incorrect, skip metric updating')


We should check and log 2 cases differently:

No metrics config: do not log anything

invalid config: log WARN message directly in validate_input_source with more information (e.g s3 metric input source missing property [file]).

Updated the code, thanks

danh-ung · 2021-09-08T12:27:55Z

+              File.open(metric_data_source.realize(params), 'r:UTF-8')
+            end
+          rescue StandardError => e
+            GoodData.logger.warn("Unable to get metric data, skip updating: #{e.message}. Error: #{e}")


Unable to get metric input source, skip updating metric formats. Error: ...

Changed this log.

danh-ung · 2021-09-08T12:28:59Z

+          expected_keys = %w[tag client_id format]
+          unless expected_keys.map(&:to_sym).all? { |s| metrics_hash.first.key? s }
+            GoodData.logger.warn("The input metric data is incorrect, expecting at least the following fields: #{expected_keys}")
+            return nil


Do we require all {expected_keys} or just one key? I suppose required all {expected_keys}

Yes, the current code is checking for all keys. I will remove "at least" to avoid confusing.

danh-ung · 2021-09-08T12:31:45Z

+
+          updated_clients = params.synchronize.map { |segment| segment.to.map { |client| client[:client_id] } }.flatten.uniq
+          data_product = params.data_product
+          data_product_clients = data_product.clients


Add execution log:

begin and end updating metrics formats.

begin and end of each project

Added logs, thanks

danh-ung · 2021-09-09T03:13:36Z

+          if params.key?('aws_client')
+            params['s3_client'] = {}
+          elsif params['input_source'] && params['input_source']['bucket'].blank?
+            if params['s3_client']['bucket'].present?


why we don't move bucket into get_s3_config(...), similar to the other fields.

The AWS connect step (Aws::S3::Resource.new(symbolized_config)) will failed if it contains 'bucket' in the parameters. Bucket shoud be specified when we download file.

danh-ung · 2021-09-09T03:36:47Z

-        s3_client = params['aws_client'] && params['aws_client']['s3_client']
+        s3_client = params['s3_client'] && params['s3_client']['client']
        raise 'AWS client not present. Perhaps S3Middleware is missing in the brick definition?' if !s3_client || !s3_client.respond_to?(:bucket)
        bucket_name = @options[:bucket]


Do we still read bucket from input source?

Yes, as in my previous answer, we have to use bucket in downloading file.

danh-ung · 2021-09-09T03:47:33Z


+      def get_s3_config(params)
+        s3_config = {}
+        if params.key?('aws_client')


We should check if both aws_client and s3_client existing and log warning message: "Found two configuration aws_client and s3_client for S3 input source, use aws_client configuration".

Added this log. Thanks

ghost · 2021-09-09T04:28:58Z

Build in pipeline check aborted.

trung-le-quoc · 2021-09-10T04:00:14Z

ok to test

ghost · 2021-09-10T04:27:07Z

Build succeeded (check pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-oss-check/875/ : SUCCESS in 19s
https://jenkins-ii.intgdc.com/job/gooddata-ruby/job/gooddata-ruby-lcm-build-pipeline/773/ : SUCCESS in 26m 23s

ghost · 2021-09-10T05:14:02Z

Build succeeded (check pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-oss-check/877/ : SUCCESS in 6s
https://jenkins-ii.intgdc.com/job/gooddata-ruby/job/gooddata-ruby-lcm-build-pipeline/775/ : SUCCESS in 23m 55s

ghost · 2021-09-10T09:03:18Z

Build failed (check pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-oss-check/880/ : SUCCESS in 7s
https://jenkins-ii.intgdc.com/job/gooddata-ruby/job/gooddata-ruby-lcm-build-pipeline/778/ : FAILURE in 21m 03s

ghost · 2021-09-10T10:48:16Z

Build in pipeline check aborted.

ghost · 2021-09-10T10:48:20Z

Merge Failed.

Zuul merger could not merge this change into the base branch. This is most likely caused by merge conflicts. Please rebase the change and upload the rebased version. In case of further errors, contact the zuul administrator.

ghost · 2021-09-10T10:51:05Z

Merge Failed.

Zuul merger could not merge this change into the base branch. This is most likely caused by merge conflicts. Please rebase the change and upload the rebased version. In case of further errors, contact the zuul administrator.

ghost · 2021-09-10T13:36:14Z

Build failed (check pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-oss-check/882/ : SUCCESS in 11s
https://jenkins-ii.intgdc.com/job/gooddata-ruby/job/gooddata-ruby-lcm-build-pipeline/779/ : FAILURE in 26m 41s

ghost · 2021-09-13T02:26:24Z

Build in pipeline check aborted.

ghost · 2021-09-13T02:47:23Z

Build succeeded (check pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-oss-check/884/ : SUCCESS in 9s
https://jenkins-ii.intgdc.com/job/gooddata-ruby/job/gooddata-ruby-lcm-build-pipeline/781/ : SUCCESS in 20m 42s

sangtm · 2021-09-13T02:48:12Z

ok to test

ghost · 2021-09-13T03:10:02Z

Build succeeded (check pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-oss-check/885/ : SUCCESS in 9s
https://jenkins-ii.intgdc.com/job/gooddata-ruby/job/gooddata-ruby-lcm-build-pipeline/782/ : SUCCESS in 21m 31s

ghost · 2021-09-22T03:28:54Z

Build failed (check pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-oss-check/902/ : SUCCESS in 7s
https://jenkins-ii.intgdc.com/job/gooddata-ruby/job/gooddata-ruby-lcm-build-pipeline/798/ : FAILURE in 3m 13s

ghost · 2021-09-22T04:33:20Z

Build succeeded (check pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-oss-check/903/ : SUCCESS in 9s
https://jenkins-ii.intgdc.com/job/gooddata-ruby/job/gooddata-ruby-lcm-build-pipeline/799/ : SUCCESS in 21m 09s

yenkins · 2021-09-22T05:11:09Z

Sonar scan result

More detail, see in https://sonarqube-gate.intgdc.com/dashboard?id=gooddata-ruby-gate-PR1800

To scan for vulnerabilities in dependencies and run unit tests (to get coverage report in sonar) please comment your PR with 'extended check sonar'.

ghost · 2021-09-22T05:30:16Z

Build succeeded (gate pipeline).

https://jenkins-ii.intgdc.com/job/gooddata-ruby/job/gooddata-ruby-lcm-promote-pipeline/151/ : SUCCESS in 20m 53s
https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-merge-compliance-check/580/ : SUCCESS in 17s
https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-sonarqube-scan-zuul-gate/38/ : SUCCESS in 1m 49s
https://checklist.intgdc.com/job/compliance/job/gooddata-ruby-fossa-scanning-zuul-el7/123/ : FAILURE in 3m 31s (non-voting)

sangtm requested a review from danh-ung August 23, 2021 11:46

sangtm added the do not merge Do not merge this yet label Aug 23, 2021

danh-ung reviewed Sep 1, 2021

View reviewed changes

sangtm force-pushed the TMA-1601-localize-metric-master branch from ba6dd35 to fbb2259 Compare September 3, 2021 10:29

sangtm force-pushed the TMA-1601-localize-metric-master branch from fbb2259 to 7843cb8 Compare September 3, 2021 10:31

danh-ung reviewed Sep 8, 2021

View reviewed changes

danh-ung reviewed Sep 9, 2021

View reviewed changes

danh-ung removed the do not merge Do not merge this yet label Sep 10, 2021

sangtm force-pushed the TMA-1601-localize-metric-master branch from 04ed476 to 11df70a Compare September 10, 2021 10:50

sangtm force-pushed the TMA-1601-localize-metric-master branch from 11df70a to 8a1a525 Compare September 10, 2021 10:55

sangtm force-pushed the TMA-1601-localize-metric-master branch from 5baa948 to 9da563d Compare September 13, 2021 02:26

danh-ung added the do not merge Do not merge this yet label Sep 14, 2021

sangtm changed the title ~~FEATURE:TMA-1601 - Update metric for client localization and bump LCM version~~ FEATURE:TMA-1061 - Update metric for client localization and bump LCM version Sep 22, 2021

sangtm changed the title ~~FEATURE:TMA-1061 - Update metric for client localization and bump LCM version~~ FEATURE:TMA-1610 - Update metric for client localization and bump LCM version Sep 22, 2021

danh-ung removed the do not merge Do not merge this yet label Sep 22, 2021

FEATURE:TMA-1610 - Update metric for client localization

f095ec8

sangtm force-pushed the TMA-1601-localize-metric-master branch from 9da563d to f095ec8 Compare September 22, 2021 03:25

FEATURE:TMA-1610 - Change redshift repos to nexus

85ee630

danh-ung added the merge label Sep 22, 2021

ghost removed the merge label Sep 22, 2021

ghost merged commit 3beaa51 into gooddata:master Sep 22, 2021

This pull request was closed.

Conversation

sangtm commented Aug 23, 2021

Uh oh!

ghost commented Aug 23, 2021

Uh oh!

ghost commented Aug 24, 2021

Uh oh!

ghost commented Aug 24, 2021

Uh oh!

sangtm commented Aug 24, 2021

Uh oh!

ghost commented Aug 24, 2021

Uh oh!

ghost commented Aug 27, 2021

Uh oh!

ghost commented Sep 1, 2021

Uh oh!

ghost commented Sep 1, 2021

Uh oh!

ghost commented Sep 1, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Sep 2, 2021

Uh oh!

ghost commented Sep 2, 2021

Uh oh!

ghost commented Sep 3, 2021

Uh oh!

ghost commented Sep 3, 2021

Uh oh!

ghost commented Sep 3, 2021

Uh oh!

ghost commented Sep 6, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Sep 9, 2021

Uh oh!

trung-le-quoc commented Sep 10, 2021

Uh oh!

ghost commented Sep 10, 2021

Uh oh!

ghost commented Sep 10, 2021