Skip to content
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

api: migrate to httpd role server #18

Merged

Conversation

themilchenko
Copy link
Contributor

Since the httpd role was released this role need to support this feature.

After the patch metrics-export-role supports an httpd role. It is possible to determine a list of servers that role will reuse in the following configuration.

Closes #15

Copy link
Contributor

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

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

The tests are red.

@themilchenko themilchenko force-pushed the themilchenko/gh-15-migrate-to-http-role-server branch 12 times, most recently from a5b80a0 to 71321be Compare September 30, 2024 12:20
Copy link
Contributor

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

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

Please, fix the comments below. We could also to improve the code and to use router:delete instead of the call:

local function delete_route(httpd, name)
local route = assert(httpd.iroutes[name])
httpd.iroutes[name] = nil
table.remove(httpd.routes, route)
-- Update httpd.iroutes numeration.
for n, r in ipairs(httpd.routes) do
if r.name then
httpd.iroutes[r.name] = n
end
end
end

CHANGELOG.md Outdated Show resolved Hide resolved
roles/metrics-export.lua Outdated Show resolved Hide resolved
roles/metrics-export.lua Outdated Show resolved Hide resolved
roles/metrics-export.lua Outdated Show resolved Hide resolved
roles/metrics-export.lua Outdated Show resolved Hide resolved
roles/metrics-export.lua Outdated Show resolved Hide resolved
roles/metrics-export.lua Outdated Show resolved Hide resolved
roles/metrics-export.lua Outdated Show resolved Hide resolved
roles/metrics-export.lua Outdated Show resolved Hide resolved
roles/metrics-export.lua Outdated Show resolved Hide resolved
@themilchenko themilchenko force-pushed the themilchenko/gh-15-migrate-to-http-role-server branch 3 times, most recently from a43e014 to ae8434a Compare September 30, 2024 17:34
@themilchenko
Copy link
Contributor Author

Please, fix the comments below. We could also to improve the code and to use router:delete instead of the call:

local function delete_route(httpd, name)
local route = assert(httpd.iroutes[name])
httpd.iroutes[name] = nil
table.remove(httpd.routes, route)
-- Update httpd.iroutes numeration.
for n, r in ipairs(httpd.routes) do
if r.name then
httpd.iroutes[r.name] = n
end
end
end

Replaced on router:delete instead of the call.

@themilchenko themilchenko force-pushed the themilchenko/gh-15-migrate-to-http-role-server branch from ae8434a to 680b4e5 Compare September 30, 2024 17:37
roles/metrics-export.lua Show resolved Hide resolved
roles/metrics-export.lua Outdated Show resolved Hide resolved
roles/metrics-export.lua Outdated Show resolved Hide resolved
roles/metrics-export.lua Outdated Show resolved Hide resolved
test/helpers/mocks.lua Show resolved Hide resolved
test/helpers/mocks.lua Outdated Show resolved Hide resolved
test/unit/validate_test.lua Outdated Show resolved Hide resolved
test/unit/validate_test.lua Outdated Show resolved Hide resolved
test/unit/validate_test.lua Outdated Show resolved Hide resolved
test/unit/http_test.lua Outdated Show resolved Hide resolved
@themilchenko themilchenko force-pushed the themilchenko/gh-15-migrate-to-http-role-server branch from 680b4e5 to 4689696 Compare October 1, 2024 07:14
Copy link
Contributor

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

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

Thank you for the patch!

README.md Outdated Show resolved Hide resolved
roles/metrics-export.lua Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
test/entrypoint/config.yaml Show resolved Hide resolved
@themilchenko themilchenko force-pushed the themilchenko/gh-15-migrate-to-http-role-server branch 2 times, most recently from 26923ee to 4d8ef4c Compare October 1, 2024 13:54
@themilchenko themilchenko force-pushed the themilchenko/gh-15-migrate-to-http-role-server branch from 4d8ef4c to 3d85ea3 Compare October 1, 2024 13:58
@themilchenko themilchenko force-pushed the themilchenko/gh-15-migrate-to-http-role-server branch from 3d85ea3 to 722abf5 Compare October 1, 2024 16:23
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Since the `httpd` role was released this role has been needed support
for this feature.

After the patch metrics-export-role supports an `httpd` role. It is
possible to determine a list of servers that role will reuse in the
following configuration.

Closes #15
@themilchenko themilchenko force-pushed the themilchenko/gh-15-migrate-to-http-role-server branch from 722abf5 to d439bcf Compare October 2, 2024 08:26
Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

Should be ready to merge.

@themilchenko themilchenko merged commit b7c6bdf into master Oct 2, 2024
3 checks passed
@themilchenko themilchenko deleted the themilchenko/gh-15-migrate-to-http-role-server branch October 2, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate to HTTP role server
3 participants