Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
functionaltests: add first test case #14935
functionaltests: add first test case #14935
Changes from 22 commits
874c46a
5ba3747
1707372
265c38f
81f1b8b
8edc826
a490d4f
999d77a
c14da87
b34e570
8747630
a031acb
bcc83e3
06dd665
e182ae3
fd73107
410fab3
991a9f1
34530df
28e468c
c48e063
e00ef0a
52a22a6
3ae84c4
d6a6146
0a02a50
db94233
b97cf14
7e6ec11
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd prefer
prd
(like the MKI cluster names) orprod
rather than usingpro
, which I've never seen used anywhere. Is it just me?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I share the preference for another acronym, I supposed this could not be changed but let's check with @v1v . Is this something that can be changed? My understanding is that
pro
is part of the secret name we use in the pipeline, but I'm not sure if it's used elsewhere or it's a secret specific for this pipeline.If it's specific, could we update it to
prd
orproduction
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have been using
pro
for a few years as far I as I recall. And we usepro
in several places and changing might imply a little bit of effort and likely some breaking changes (so others will need to change it too). However let me cc @kuisathaveratThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sound pretty complex and time consuming. I have an easy workaround that could be applied in the GitHub Workflow itself: use an
if
onmatrix.environment
to trigger the secret retrieval with a different value. Low effort and would solve the main confusion, so I would proceed with it instead of having an extended effort to fix it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use everywhere
pro
,qa
, andstaging
, changing it on the secrets meant we have to change it everywhere (oblt-framework, oblt-cli, oblt-robot, observability-test-environments) to be consistentThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then let's settle on using it only for the secret (via
ifs
in the pipeline) then use a more standard naming in the code. Not a big deal once it's documented. I'll take care of it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this called
ac
? Shouldn't it be calledec
,esc
oresClient
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed in 52a22a6, this was a leftover. Choose
ecc
as Elastic Cloud client.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could set up monitoring to self and the ES logs should end up in an index in this same cluster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 great idea, will look into it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worked like a charm, implemented in 0a02a50 and db94233