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

split off geoserver functionality into a microservice - update docker… #4427

Merged
merged 9 commits into from
Nov 14, 2024

Conversation

devinleighsmith
Copy link
Collaborator

… and ci/cd to support.

@devinleighsmith devinleighsmith added enhancement New feature or request 5.7 labels Oct 31, 2024
@devinleighsmith devinleighsmith self-assigned this Oct 31, 2024
@devinleighsmith
Copy link
Collaborator Author

Please don't review until after 5.6 functionality is complete

@devinleighsmith devinleighsmith changed the base branch from 5.7 to dev November 8, 2024 00:59
Copy link
Contributor

github-actions bot commented Nov 8, 2024

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

1 similar comment
Copy link
Contributor

github-actions bot commented Nov 8, 2024

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

Comment on lines 29 to 31
pull_request_target:
branches: [dev]
types: [closed]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this change but it will lead to a change in our process. From now on, developers will be responsible to trigger the CI build for DEV environment themselves, right?
We should talk about how would that look like within the team

docker-compose.yml Outdated Show resolved Hide resolved
@@ -1 +0,0 @@
DOTNET_STARTUP_PROJECT=api/Pims.Api.csproj
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this file is used as part of openshift building .NET
Did you test this change in openshift?

Comment on lines 30 to 32
COPY entrypoint.sh .
RUN chmod +x /app/entrypoint.proxy.sh
ENTRYPOINT ["/app/entrypoint.proxy.sh"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you check that we are copying the right file?

COPY entrypoint.proxy.sh . 
# instead of entrypoint.sh which does not exist on this folder?

Comment on lines -26 to -43

/// <summary>
/// Parses the environment configuration and returns 'JsonSerializerOptions'.
/// </summary>
/// <param name="configuration"></param>
/// <returns></returns>
public static JsonSerializerOptions GenerateJsonSerializerOptions(this IConfiguration configuration)
{
return new JsonSerializerOptions()
{
DefaultIgnoreCondition = (!string.IsNullOrWhiteSpace(configuration["Serialization:Json:IgnoreNullValues"]) && bool.Parse(configuration["Serialization:Json:IgnoreNullValues"])) ? JsonIgnoreCondition.WhenWritingNull : JsonIgnoreCondition.Never,
PropertyNameCaseInsensitive = !string.IsNullOrWhiteSpace(configuration["Serialization:Json:PropertyNameCaseInsensitive"]) && bool.Parse(configuration["Serialization:Json:PropertyNameCaseInsensitive"]),
PropertyNamingPolicy = configuration["Serialization:Json:PropertyNamingPolicy"] == "CamelCase" ? JsonNamingPolicy.CamelCase : null,
WriteIndented = !string.IsNullOrWhiteSpace(configuration["Serialization:Json:WriteIndented"]) && bool.Parse(configuration["Serialization:Json:WriteIndented"]),
Converters = { new JsonStringEnumMemberConverter(JsonNamingPolicy.CamelCase) },
ReferenceHandler = ReferenceHandler.IgnoreCycles,
};
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm guessing this was dead-code?

@@ -1,4 +1,6 @@
using Pims.Core.Security;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: github is complaining here about linting. perhaps need to sort the using statements

Copy link
Contributor

github-actions bot commented Nov 9, 2024

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

10 similar comments
Copy link
Contributor

github-actions bot commented Nov 9, 2024

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

Copy link
Contributor

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

Copy link
Contributor

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

Copy link
Contributor

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

Copy link
Contributor

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

Copy link
Contributor

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

Copy link
Contributor

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

Copy link
Contributor

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

Copy link
Contributor

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

Copy link
Contributor

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

Copy link
Contributor

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

1 similar comment
Copy link
Contributor

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4427

@devinleighsmith devinleighsmith merged commit 7a53d28 into bcgov:dev Nov 14, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants